diff --git a/.dockerignore b/.dockerignore index 9558038828..a3cd4575df 100644 --- a/.dockerignore +++ b/.dockerignore @@ -4,3 +4,10 @@ docker-compose.yml .git .github .gitignore +.semgrep-env/ +trivy-*.txt +.env +.env.* +artifacts/cert/server.key +node_modules +artifacts/cert/server.key.backup diff --git a/.env.example b/.env.example new file mode 100644 index 0000000000..8025a0a7ce --- /dev/null +++ b/.env.example @@ -0,0 +1,2 @@ +MONGODB_URI=mongodb://USERNAME:PASSWORD@HOST:27017/nodegoat +SESSION_SECRET=replace-with-real-secret diff --git a/.github/workflows/devsecops.yml b/.github/workflows/devsecops.yml new file mode 100644 index 0000000000..3da4787ab0 --- /dev/null +++ b/.github/workflows/devsecops.yml @@ -0,0 +1,113 @@ +name: DevSecOps Pipeline + +on: + push: + pull_request: + +jobs: + build-and-test: + name: Build and Unit Test + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version: 22 + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Run unit tests + run: npm test + + dependency-security: + name: Dependency Security Audit (report only) + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version: 22 + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Dependency Security Audit + continue-on-error: true + run: npm audit --omit=dev --audit-level=high + + sast-semgrep: + name: SAST - Semgrep + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.12" + + - name: Install Semgrep + run: python -m pip install semgrep + + - name: Run Semgrep SAST + run: | + semgrep scan \ + --config auto \ + --config .semgrep.yml \ + --json \ + --output semgrep-results.json . + + python - <<'PY' + import json + with open("semgrep-results.json") as f: + data = json.load(f) + print(f"Semgrep findings: {len(data.get('results', []))}") + PY + + gitleaks: + name: Secret Scanning - Gitleaks + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Run Gitleaks + uses: gitleaks/gitleaks-action@v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + trivy: + name: Container Security - Trivy + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Build Docker image + run: docker build -t nodegoat-member4-web:${{ github.sha }} . + + - name: Run Trivy vulnerability scan + uses: aquasecurity/trivy-action@v0.36.0 + with: + image-ref: nodegoat-member4-web:${{ github.sha }} + format: table + vuln-type: os,library + severity: HIGH,CRITICAL + exit-code: '1' \ No newline at end of file diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 4ed7d6aec1..33e7dd24a5 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -1,4 +1,5 @@ name: E2E Test + on: [push, pull_request] jobs: @@ -9,21 +10,21 @@ jobs: strategy: fail-fast: false matrix: - node-version: ["10.x", "12.x", "14.x"] + node-version: ["22.x"] steps: - name: Checkout https://github.com/${{ github.repository }}@${{ github.ref }} - uses: actions/checkout@v2 + uses: actions/checkout@v6 with: persist-credentials: false - name: Set up Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v7 with: node-version: ${{ matrix.node-version }} - name: Use cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: | ~/.npm @@ -37,15 +38,37 @@ jobs: - name: Start MongoDB run: | - docker run -d -p 27017:27017 mongo:4.0 - timeout 60s bash -c 'until nc -z -w 2 localhost 27017 && echo MongoDB ready; do sleep 2; done' + docker run -d --name nodegoat-mongo -p 27017:27017 mongo:4.4 + for attempt in {1..30}; do + if docker exec nodegoat-mongo mongo --quiet --eval 'db.adminCommand({ ping: 1 }).ok' | grep -qx 1; then + echo "MongoDB ready" + exit 0 + fi + sleep 2 + done + docker logs nodegoat-mongo + exit 1 + + - name: Seed test database + run: npm run db:seed - name: Run E2E test suite id: test-suite run: | - NODE_ENV=test npm start -- --silent & + NODE_ENV=test npm start -- --silent > /tmp/nodegoat.log 2>&1 & + for attempt in $(seq 1 30); do + if curl --silent --fail http://localhost:4000/login >/dev/null; then + break + fi + sleep 2 + done + curl --silent --fail http://localhost:4000/login >/dev/null || { cat /tmp/nodegoat.log; exit 1; } npm run test:ci -- --config video=true + - name: Show application logs on failure + if: failure() && (steps.test-suite.outcome == 'failure') + run: cat /tmp/nodegoat.log + - name: Prepare cypress artifacts if: failure() && (steps.test-suite.outcome == 'failure') working-directory: ./test/e2e @@ -55,7 +78,7 @@ jobs: - name: Upload cypress artifacts if: failure() && (steps.test-suite.outcome == 'failure') - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: cypress-artifacts-node${{ matrix.node-version }} - path: test/e2e/screenshots + path: test/e2e/screenshots \ No newline at end of file diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e7922ae780..4bc9fac791 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -9,18 +9,21 @@ jobs: strategy: fail-fast: false matrix: - node-version: ["14.x"] + node-version: ["22.x"] steps: - name: Checkout https://github.com/${{ github.repository }}@${{ github.ref }} - uses: actions/checkout@v2 + uses: actions/checkout@v6 with: persist-credentials: false - name: Set up Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v1 + uses: actions/setup-node@v7 with: node-version: ${{ matrix.node-version }} + - name: Install dependencies + run: npm ci + - name: Run linter - run: npx --no-install jshint@2.12.0 . + run: npx --no-install jshint . diff --git a/.gitignore b/.gitignore index 4cd536b01f..9d95e992f3 100644 --- a/.gitignore +++ b/.gitignore @@ -24,8 +24,12 @@ test/e2e/screenshots/ test/e2e/videos/ # ignore sensitive files -.env.local .env +.env.* +!.env.example +artifacts/cert/server.key.backup # ignore Snyk Code scanner files .dccache + +/scan-comparison/ diff --git a/.semgrep.yml b/.semgrep.yml new file mode 100644 index 0000000000..c06feab666 --- /dev/null +++ b/.semgrep.yml @@ -0,0 +1,7 @@ +rules: + - id: nodegoat-eval-user-input + languages: + - javascript + message: "User-controlled input is passed to eval(). Use numeric parsing instead." + severity: ERROR + pattern: eval(req.body.$FIELD) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 0000000000..13ad5e01cd --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,158 @@ +\# NodeGoat Architecture + + + +\## Overview + + + +The application runs locally using Docker Compose with two services: + + + +| Service | Purpose | Container port | + +|---|---|---| + +| web | NodeGoat web application using Node.js and Express | 4000 | + +| mongo | MongoDB database storing application data | 27017 | + + + +The web image is built from the project's Dockerfile. + +The database uses the mongo:4.4 image. + + + +\## Architecture diagram + + + +```mermaid + +flowchart LR + + subgraph HOST\["Windows host"] + + B\["Browser - untrusted user input"] + + P\["Loopback address - 127.0.0.1:4000"] + + + + subgraph NET\["Docker Compose internal network"] + + W\["web container - NodeGoat / Express - port 4000"] + + M\[("mongo container - MongoDB 4.4 - port 27017")] + + end + + + + B -->|"HTTP requests - trust boundary 1"| P + + P -->|"Port forwarding"| W + + W -->|"HTTP responses"| B + + W -->|"Database operations - trust boundary 2"| M + + M -->|"Query results"| W + + end + +``` + + + +\## Data flows + + + +1\. A user opens http://localhost:4000 in their browser. + +2\. Docker forwards traffic from host address 127.0.0.1:4000 to port 4000 in the web container. + +3\. NodeGoat processes the request. + +4\. When database access is needed, NodeGoat connects to mongodb://mongo:27017/nodegoat. + +5\. Docker resolves the service name mongo within the Compose network. + +6\. MongoDB returns results to NodeGoat. + +7\. NodeGoat sends an HTTP response to the browser. + + + +\## Trust boundaries + + + +\### Boundary 1: Browser to application + + + +Browser requests are user-controlled and must be treated as untrusted. + +Input validation, authentication and authorisation controls belong at this boundary. + + + +\### Boundary 2: Application to database + + + +The application sends queries and updates to persistent application data. + +Safe query construction and appropriate database access controls are required at this boundary. + + + +These are required security controls to assess, not claims that the original application implements them correctly. + + + +\## Network exposure + + + +\- The web port is published as 127.0.0.1:4000:4000. + +\- The published web port is restricted to the host loopback address. + +\- MongoDB port 27017 is not published to the Windows host. + +\- The web service reaches MongoDB through the Compose internal network. + +\- There is no direct browser-to-database connection in this architecture. + + + +\## Current setup limitations + + + +\- The local application uses HTTP; this configuration does not provide HTTPS. + +\- The Compose startup command invokes artifacts/db-reset.js before starting NodeGoat, so startup can reset sample data. + +\- The Compose file does not explicitly configure a named database volume or a host directory for database storage. + +\- The original application is intentionally vulnerable; containerisation alone does not fix its application vulnerabilities. + + + +\## Source + + + +This diagram describes our project's Dockerfile and docker-compose.yml. + +The original application is OWASP NodeGoat: + +https://github.com/OWASP/NodeGoat + diff --git a/Dockerfile b/Dockerfile index 91a5b43e17..74a2a6fa57 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,18 +1,31 @@ -FROM node:12-alpine +FROM node:22-alpine AS dependencies ENV WORKDIR /usr/src/app/ WORKDIR $WORKDIR COPY package*.json $WORKDIR -RUN npm install --production --no-cache +RUN npm ci --omit=dev -FROM node:12-alpine +FROM node:22-alpine +RUN apk add --no-cache ca-certificates \ + && rm -rf /usr/local/lib/node_modules/npm \ + /usr/local/lib/node_modules/corepack \ + /opt/yarn-v1.22.22 \ + && rm -f /usr/local/bin/npm \ + /usr/local/bin/npx \ + /usr/local/bin/corepack \ + /usr/local/bin/yarn \ + /usr/local/bin/yarnpkg ENV USER node ENV WORKDIR /home/$USER/app WORKDIR $WORKDIR -COPY --from=0 /usr/src/app/node_modules node_modules +COPY --from=dependencies /usr/src/app/node_modules node_modules RUN chown $USER:$USER $WORKDIR -COPY --chown=node . $WORKDIR +COPY --chown=node app/ app/ +COPY --chown=node config/ config/ +COPY --chown=node artifacts/db-reset.js artifacts/db-reset.js +COPY --chown=node server.js server.js # In production environment uncomment the next line #RUN chown -R $USER:$USER /home/$USER && chmod -R g-s,o-rx /home/$USER && chmod -R o-wrx $WORKDIR # Then all further actions including running the containers should be done under non-root user. USER $USER EXPOSE 4000 +CMD ["node", "server.js"] diff --git a/SETUP.md b/SETUP.md new file mode 100644 index 0000000000..f383d40b55 --- /dev/null +++ b/SETUP.md @@ -0,0 +1,182 @@ +\# NodeGoat Local Setup + + + +\## Requirements + + + +\- Git + +\- Docker Desktop, running with Linux containers + +\- Internet access for the initial image and dependency downloads + + + +\## Download the project + + + +```powershell + +git clone https://github.com/Thakshila05/NodeGoat.git + +cd NodeGoat + +git switch member1/docker-setup + +``` + + + +The setup changes are currently on the member1/docker-setup branch. + + + +\## Build and start + + + +Run from the folder containing docker-compose.yml: + + + +```powershell + +docker compose up --build -d + +``` + + + +This builds the NodeGoat application image and starts the web and MongoDB services in the background. + + + +\## Check the services + + + +```powershell + +docker compose ps + +``` + + + +Both services should show an Up status. + + + +The web service should display: + + + +```text + +127.0.0.1:4000->4000/tcp + +``` + + + +\## Open the application + + + +Open http://localhost:4000 in a browser. + + + +Use only sample data and training accounts in this application. + + + +\## View recent logs + + + +```powershell + +docker compose logs --tail=50 + +``` + + + +\## Stop the application + + + +```powershell + +docker compose stop + +``` + + + +This stops the containers without removing them. + + + +\## Start the application again + + + +```powershell + +docker compose up -d + +``` + + + +The existing startup command runs a database-reset script, so restarting the web service can reset sample data. + + + +\## Architecture + + + +\- web: NodeGoat application built from the project Dockerfile. + +\- mongo: MongoDB service using the mongo:4.4 image. + +\- NodeGoat connects to mongodb://mongo:27017/nodegoat through the Compose network. + +\- The application is published on the host loopback address at port 4000. + +\- MongoDB port 27017 is not published to the host. + + + +\## Original baseline + + + +Starting commit recorded before our changes: + + + +c5cb68a7084e4ae7dcc60e6a98768720a81841e8 + + + +Original project: https://github.com/OWASP/NodeGoat + + + +\## Troubleshooting + + + +\- If Docker cannot connect to its engine, open Docker Desktop and wait for it to start. + +\- If the browser cannot connect, check docker compose ps and the recent logs. + +\- An obsolete version-field warning does not by itself mean startup failed. + diff --git a/app/routes/allocations.js b/app/routes/allocations.js index 616f717b69..bb7b24ec2d 100644 --- a/app/routes/allocations.js +++ b/app/routes/allocations.js @@ -9,13 +9,11 @@ function AllocationsHandler(db) { const allocationsDAO = new AllocationsDAO(db); this.displayAllocations = (req, res, next) => { - /* - // Fix for A4 Insecure DOR - take user id from session instead of from URL param - const { userId } = req.session; - */ - const { + + const { userId - } = req.params; + } = req.session; + const { threshold } = req.query; diff --git a/app/routes/contributions.js b/app/routes/contributions.js index 7f68170b94..0b46f6e5dc 100644 --- a/app/routes/contributions.js +++ b/app/routes/contributions.js @@ -27,24 +27,25 @@ function ContributionsHandler(db) { this.handleContributionsUpdate = (req, res, next) => { - /*jslint evil: true */ - // Insecure use of eval() to parse inputs - const preTax = eval(req.body.preTax); - const afterTax = eval(req.body.afterTax); - const roth = eval(req.body.roth); - - /* - //Fix for A1 -1 SSJS Injection attacks - uses alternate method to eval - const preTax = parseInt(req.body.preTax); - const afterTax = parseInt(req.body.afterTax); - const roth = parseInt(req.body.roth); - */ + + // Convert contribution input to numbers without executing it as code. + const preTax = Number(req.body.preTax); + const afterTax = Number(req.body.afterTax); + const roth = Number(req.body.roth); + const { userId } = req.session; //validate contributions - const validations = [isNaN(preTax), isNaN(afterTax), isNaN(roth), preTax < 0, afterTax < 0, roth < 0]; + const validations = [ + !Number.isFinite(preTax), + !Number.isFinite(afterTax), + !Number.isFinite(roth), + preTax < 0, + afterTax < 0, + roth < 0 + ]; const isInvalid = validations.some(validation => validation); if (isInvalid) { return res.render("contributions", { diff --git a/app/routes/index.js b/app/routes/index.js index a9e55426bf..5fc837e917 100644 --- a/app/routes/index.js +++ b/app/routes/index.js @@ -68,8 +68,13 @@ const index = (app, db) => { // Handle redirect for learning resources link app.get("/learn", isLoggedIn, (req, res) => { - // Insecure way to handle redirects by taking redirect url from query string - return res.redirect(req.query.url); + const allowedUrl = "/tutorial"; + + if (req.query.url === allowedUrl) { + return res.redirect(allowedUrl); + } + + return res.status(400).send("Invalid redirect URL"); }); // Research Page diff --git a/app/routes/profile.js b/app/routes/profile.js index 0b5b34f2dd..67ac144aa6 100644 --- a/app/routes/profile.js +++ b/app/routes/profile.js @@ -56,7 +56,7 @@ function ProfileHandler(db) { // -- // The Fix: Instead of using greedy quantifiers the same regex will work if we omit the second quantifier + // const regexPattern = /([0-9]+)\#/; - const regexPattern = /([0-9]+)+\#/; + const regexPattern = /([0-9]+)\#/; // Allow only numbers with a suffix of the letter #, for example: 'XXXXXX#' const testComplyWithRequirements = regexPattern.test(bankRouting); // if the regex test fails we do not allow saving diff --git a/app/views/layout.html b/app/views/layout.html index 380ba414b0..56e07ec8a8 100644 --- a/app/views/layout.html +++ b/app/views/layout.html @@ -74,7 +74,7 @@
  • {{firstName}} {{lastName}}