From b7565680af1e01ff275be68dafd4ccd517a99a06 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Wed, 18 Sep 2024 11:17:28 +0200 Subject: [PATCH 01/28] Be consistent with using .fixed.test.yaml not .test.fixed.yaml (#3471) test plan: make test --- ...s.test.fixed.yaml => no-fractional-cpu-limits.fixed.test.yaml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename yaml/kubernetes/best-practice/{no-fractional-cpu-limits.test.fixed.yaml => no-fractional-cpu-limits.fixed.test.yaml} (100%) diff --git a/yaml/kubernetes/best-practice/no-fractional-cpu-limits.test.fixed.yaml b/yaml/kubernetes/best-practice/no-fractional-cpu-limits.fixed.test.yaml similarity index 100% rename from yaml/kubernetes/best-practice/no-fractional-cpu-limits.test.fixed.yaml rename to yaml/kubernetes/best-practice/no-fractional-cpu-limits.fixed.test.yaml From 6d1b466dae4fe7111b91b989e2896e1fe9a1a3b7 Mon Sep 17 00:00:00 2001 From: Sjoerd Langkemper Date: Thu, 19 Sep 2024 08:43:26 +0200 Subject: [PATCH 02/28] PHP tainted exec (#3468) * PHP tainted exec When user input is passed to a function that executes a shell command, without escaping. * Correct message string YAML operator Co-authored-by: Pieter De Cremer (Semgrep) --------- Co-authored-by: Pieter De Cremer (Semgrep) Co-authored-by: Lewis --- php/lang/security/injection/tainted-exec.php | 21 ++++++++ php/lang/security/injection/tainted-exec.yaml | 51 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 php/lang/security/injection/tainted-exec.php create mode 100644 php/lang/security/injection/tainted-exec.yaml diff --git a/php/lang/security/injection/tainted-exec.php b/php/lang/security/injection/tainted-exec.php new file mode 100644 index 0000000000..9f4c123250 --- /dev/null +++ b/php/lang/security/injection/tainted-exec.php @@ -0,0 +1,21 @@ +- + User input is passed to a function that executes a shell command. This can lead to remote code execution. + metadata: + cwe: + - "CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')" + category: security + technology: + - php + owasp: + - A03:2021 - Injection + references: + - https://owasp.org/Top10/A03_2021-Injection + subcategory: + - vuln + impact: HIGH + likelihood: MEDIUM + confidence: MEDIUM + mode: taint + pattern-sources: + - patterns: + - pattern-either: + - pattern: $_GET + - pattern: $_POST + - pattern: $_COOKIE + - pattern: $_REQUEST + - pattern: file_get_contents('php://input') + pattern-sanitizers: + - patterns: + - pattern-either: + - pattern: escapeshellcmd(...) + - pattern: escapeshellarg(...) + pattern-sinks: + - patterns: + - pattern-either: + - pattern: exec(...) + - pattern: system(...) + - pattern: passthru(...) + - patterns: + - pattern: proc_open(...) + - pattern-not: proc_open([...], ...) + - pattern: popen(...) + - pattern: expect_popen(...) + - pattern: shell_exec(...) + - pattern: | + `...` + From 7427b8245eb9c4044cf7fa54bd5fd414fb827103 Mon Sep 17 00:00:00 2001 From: Hardik Nanda Date: Thu, 19 Sep 2024 12:26:50 +0530 Subject: [PATCH 03/28] Upload dockerd socket mount detection rule and test file (#3360) * Upload dockerd socket mount detection rule and test file * Update dockerd-socket-mount.dockerfile * Update documentbuilderfactory-disallow-doctype-decl-missing.yaml Update the rule for checking if FEATURE_SECURE_PROCESSING is set to TRUE for DocumentBuilderFactory object. * Revert "Update documentbuilderfactory-disallow-doctype-decl-missing.yaml" This reverts commit c1e22817279cfe141717e80458f48f7eff8ca71a. --------- Co-authored-by: Pieter De Cremer (Semgrep) --- .../security/dockerd-socket-mount.dockerfile | 11 ++++++ dockerfile/security/dockerd-socket-mount.yaml | 36 +++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 dockerfile/security/dockerd-socket-mount.dockerfile create mode 100644 dockerfile/security/dockerd-socket-mount.yaml diff --git a/dockerfile/security/dockerd-socket-mount.dockerfile b/dockerfile/security/dockerd-socket-mount.dockerfile new file mode 100644 index 0000000000..a1b0574974 --- /dev/null +++ b/dockerfile/security/dockerd-socket-mount.dockerfile @@ -0,0 +1,11 @@ +FROM docker:latest + +WORKDIR /app + +# ruleid: dockerfile-dockerd-socket-mount +VOLUME /var/run/docker.sock:/var/run/docker.sock + +# ok: dockerfile-dockerd-socket-mount +VOLUME ./app/main.py:/main.py + +CMD ["docker", "images"] diff --git a/dockerfile/security/dockerd-socket-mount.yaml b/dockerfile/security/dockerd-socket-mount.yaml new file mode 100644 index 0000000000..49a3c34ebc --- /dev/null +++ b/dockerfile/security/dockerd-socket-mount.yaml @@ -0,0 +1,36 @@ +rules: + - id: dockerfile-dockerd-socket-mount + message: >- + The Dockerfile(image) mounts docker.sock to the container which may allow an attacker already inside of the container + to escape container and execute arbitrary commands on the host machine. + languages: + - dockerfile + - yaml + severity: ERROR + metadata: + cwe: + - "CWE-862: Missing Authorization" + - "CWE-269: Improper Privilege Management" + confidence: HIGH + likelihood: MEDIUM + impact: HIGH + subcategory: + - audit + technology: + - dockerfile + category: security + references: + - https://cheatsheetseries.owasp.org/cheatsheets/Docker_Security_Cheat_Sheet.html + - https://redfoxsec.com/blog/insecure-volume-mounts-in-docker/ + - https://blog.quarkslab.com/why-is-exposing-the-docker-socket-a-really-bad-idea.html + pattern-either: + - patterns: + - pattern: VOLUME $X + - metavariable-regex: + metavariable: $X + regex: "/var/run/docker.sock" + - patterns: + - pattern-regex: '- "/var/run/docker.sock:.*"' + - pattern-inside: | + volumes: + ... From be389acffa5d5f89a4034fff8064930069428b2e Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Thu, 19 Sep 2024 09:10:34 +0200 Subject: [PATCH 04/28] Switch to osemgrep test --experimental (from 3min to 21s) (#3472) * Switch to osemgrep test --experimental test plan: wait for green CI check * comment * comments --- .../workflows/semgrep-rules-test-develop.yml | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index 190bb44484..b3c85c01a6 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -1,3 +1,6 @@ +# Running the tests in the repo using `semgrep test --experimental` and +# the semgrep/semgrep:pro-develop docker image (the bleeding edge!). + name: semgrep-rules-test-develop on: pull_request: @@ -9,8 +12,6 @@ on: - develop - release jobs: - # Note: if you change this test there will likely need to be a - # corresponding change in returntocorp/semgrep test-develop: name: rules-test-develop runs-on: ubuntu-20.04 @@ -18,17 +19,20 @@ jobs: - uses: actions/checkout@v2 with: path: semgrep-rules - - name: delete stats directory - run: rm -rf semgrep-rules/stats - - name: delete fingerprints directory - run: rm -rf semgrep-rules/fingerprints + - name: delete directories not containing rules + run: rm -rf semgrep-rules/stats semgrep-rules/fingerprints + #TODO: in theory we could run those tests by using --test --pro + # since our docker image now contains semgrep-core-pro (but + # it would require to be logged in though via a SEMGREP_APP_TOKEN) - name: delete rules requiring Semgrep Pro run: rm -rf semgrep-rules/apex semgrep-rules/elixir + # TODO: this takes 1m20 in CI and could be optimized by switching to osemgrep - name: validate rules run: | - export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src returntocorp/semgrep:develop semgrep" + export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep" make -C "$GITHUB_WORKSPACE"/semgrep-rules validate - - name: test with semgrep develop branch + # this now takes 21s with osemgrep instead of 3min with pysemgrep + - name: test with semgrep pro develop branch and with --experimental run: | - export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src returntocorp/semgrep:develop semgrep" + export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep --experimental" make -C "$GITHUB_WORKSPACE"/semgrep-rules test-only From 46fc3400ded262aba7306909928672f1ce70428b Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Thu, 19 Sep 2024 16:35:52 +0200 Subject: [PATCH 05/28] remove fingerprints/fingerprints.yaml (#3474) * remove fingerprints/fingerprints.yaml No idea what this file is, but it's annoying because we have to skip it in many scripts because it does not contain regular rules and target test files. Let's just remove it to simplify things. test plan: wait for green CI checks * remove every use of fingerprints (each time it was to skip the dir) --- .codemapignore | 1 - .github/workflows/semgrep-rule-lints.yaml | 1 - .../workflows/semgrep-rules-test-develop.yml | 2 +- .../semgrep-rules-test-historical.yml | 2 - .github/workflows/semgrep-rules-test.yml | 2 - ...trigger-semgrep-scanner-initiate-scan.yaml | 2 +- fingerprints/fingerprints.yaml | 826 ------------------ scripts/run-tests | 2 +- 8 files changed, 3 insertions(+), 835 deletions(-) delete mode 100644 fingerprints/fingerprints.yaml diff --git a/.codemapignore b/.codemapignore index 0d77320097..4d725e3c70 100644 --- a/.codemapignore +++ b/.codemapignore @@ -10,7 +10,6 @@ !libsonnet/ !scripts/ !stats/ -# restore also fingerprints/ ? trusted_python/ ? # do not skip the rules ![a-z]*/**/*.yaml diff --git a/.github/workflows/semgrep-rule-lints.yaml b/.github/workflows/semgrep-rule-lints.yaml index 9114b485ac..034af3014c 100644 --- a/.github/workflows/semgrep-rule-lints.yaml +++ b/.github/workflows/semgrep-rule-lints.yaml @@ -45,5 +45,4 @@ jobs: --exclude *.test.yaml \ --exclude contrib/ \ --exclude stats/ \ - --exclude fingerprints/ \ --exclude yaml/semgrep/ diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index b3c85c01a6..50fff01293 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -20,7 +20,7 @@ jobs: with: path: semgrep-rules - name: delete directories not containing rules - run: rm -rf semgrep-rules/stats semgrep-rules/fingerprints + run: rm -rf semgrep-rules/stats #TODO: in theory we could run those tests by using --test --pro # since our docker image now contains semgrep-core-pro (but # it would require to be logged in though via a SEMGREP_APP_TOKEN) diff --git a/.github/workflows/semgrep-rules-test-historical.yml b/.github/workflows/semgrep-rules-test-historical.yml index 55ecab1ac3..071ef8271f 100644 --- a/.github/workflows/semgrep-rules-test-historical.yml +++ b/.github/workflows/semgrep-rules-test-historical.yml @@ -32,8 +32,6 @@ jobs: run: pip3 install semgrep - name: delete stats directory run: rm -rf semgrep-rules/stats - - name: delete fingerprints directory - run: rm -rf semgrep-rules/fingerprints - name: delete rules requiring Semgrep Pro run: rm -rf semgrep-rules/apex semgrep-rules/elixir # TODO: remove this in the future, there was a regression in semgrep that diff --git a/.github/workflows/semgrep-rules-test.yml b/.github/workflows/semgrep-rules-test.yml index 26270ef468..ac9672914e 100644 --- a/.github/workflows/semgrep-rules-test.yml +++ b/.github/workflows/semgrep-rules-test.yml @@ -21,8 +21,6 @@ jobs: run: pip3 install semgrep - name: remove stats directory run: rm -rf stats - - name: remove fingerprints from testing - run: rm -rf fingerprints - name: remove rules requiring Semgrep Pro run: rm -rf apex elixir - name: validate rules diff --git a/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml b/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml index 91aff30daa..0c29a8c1fe 100644 --- a/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml +++ b/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml @@ -21,7 +21,7 @@ jobs: env: HEAD_REF: ${{ github.head_ref }} run: | - CHANGED_FILES=$(git diff --name-only origin/develop origin/$HEAD_REF | xargs -0 | sed '/^$/d' | sed -r '/^(.github|bash|contrib|fingerprints|generic|json|problem-based-packs|scripts|stats|trusted_python|yaml)/d' | sed -n '/.*yaml/p' | tr '\n' ' ') + CHANGED_FILES=$(git diff --name-only origin/develop origin/$HEAD_REF | xargs -0 | sed '/^$/d' | sed -r '/^(.github|bash|generic|json|problem-based-packs|scripts|stats|trusted_python|yaml)/d' | sed -n '/.*yaml/p' | tr '\n' ' ') echo "changed_files=$CHANGED_FILES" >> $GITHUB_ENV - id: print-changed-files name: debugging step - print changed files diff --git a/fingerprints/fingerprints.yaml b/fingerprints/fingerprints.yaml deleted file mode 100644 index 96472bb79a..0000000000 --- a/fingerprints/fingerprints.yaml +++ /dev/null @@ -1,826 +0,0 @@ -rules: - - id: cai-generic-technology-docker-compose - languages: - - generic - message: - Detected a docker-compose.yml file. This likely indicates that Docker and - docker-compose are in use. - metadata: - license: Semgrep Registry License - paths: - include: - - docker-compose.yml - pattern-regex: ^. - severity: INVENTORY - - id: cai-generic-technology-dockerfile - languages: - - generic - message: Detected a Dockerfile file. This likely indicates that Docker is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - Dockerfile - pattern-regex: ^. - severity: INVENTORY - - id: cai-generic-technology-kubernetes - languages: - - generic - message: - Detected a Kubernetes configuration file. This likely indicates that Kubernetes - is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.yaml" - - "*.yml" - pattern: "apiVersion: ...\nkind: ...\nmetadata:\n ...\nspec:\n ...\n" - severity: INVENTORY - - id: cai-generic-technology-travis - languages: - - generic - message: - Detected a .travis.yml file. This likely indicates that TravisCI is in - use. - metadata: - license: Semgrep Registry License - paths: - include: - - .travis.yml - pattern-regex: ^. - severity: INVENTORY - - id: cai-generic-technology-circleci - languages: - - generic - message: Detected a CircleCI file. This likely indicates that CircleCI is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - .circleci/config.yml - - circle.yml - pattern-regex: ^. - severity: INVENTORY - - id: cai-generic-technology-github-actions - languages: - - generic - message: - Detected a Github Actions file. This likely indicates that Github Actions - is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - .github/workflows/*.yaml - - .github/workflows/*.yml - pattern-regex: ^. - severity: INVENTORY - - id: cai-generic-technology-gitlab - languages: - - generic - message: Detected a GitLab file. This likely indicates that GitLab is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - .gitlab-ci.yml - pattern-regex: ^. - severity: INVENTORY - - id: cai-java-dependency-spring - languages: - - generic - message: "Detected `spring` as a dependency in a pom.xml file. - - This likely indicates that Spring framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - pom.xml - patterns: - - pattern: - "\n ...\n spring-...\n \ - \ ...\n\n" - - pattern-not-inside: "\n ...\n\n" - severity: INVENTORY - - id: cai-java-framework-spring - languages: - - java - message: - Detected `spring` being imported, indicating this app uses the Java spring - web framework - metadata: - license: Semgrep Registry License - pattern: "import org.springframework.$PATH; - - " - severity: INVENTORY - - id: cai-javascript-dependency-angular-core-yarn - languages: - - generic - message: "Detected `angular-core` as a dependency in a yarn.lock file. - - This likely indicates that Angular frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: "@angular/core@$VERSION" - - pattern: "@angular/core@^$VERSION" - - pattern: "@angular/core@~$VERSION" - - pattern: "@angular/core@>$VERSION" - - pattern: "@angular/core@<$VERSION" - - pattern: "@angular/core@>=$VERSION" - - pattern: "@angular/core@<=$VERSION" - severity: INVENTORY - - id: cai-javascript-dependency-angular-core - languages: - - json - message: "Detected `angular-core` as a dependency in a package.json file. - - This likely indicates that Angular frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"@angular/core\"\ - : $VERSION\n }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"@angular/core\"\ - : $VERSION\n }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-angular-yarn - languages: - - generic - message: "Detected `angular` as a dependency in a yarn.lock file. - - This likely indicates that Angular frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: angular@$VERSION - - pattern: angular@^$VERSION - - pattern: angular@~$VERSION - - pattern: angular@>$VERSION - - pattern: angular@<$VERSION - - pattern: angular@>=$VERSION - - pattern: angular@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-angular - languages: - - json - message: "Detected `angular` as a dependency in a package.json file. - - This likely indicates that Angular frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"angular\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"angular\": $VERSION\n\ - \ }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-vue-yarn - languages: - - generic - message: "Detected `vue` as a dependency in a yarn.lock file. - - This likely indicates that vue - a JS MVC framework. is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: vue@$VERSION - - pattern: vue@^$VERSION - - pattern: vue@~$VERSION - - pattern: vue@>$VERSION - - pattern: vue@<$VERSION - - pattern: vue@>=$VERSION - - pattern: vue@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-vue - languages: - - json - message: "Detected `vue` as a dependency in a package.json file. - - This likely indicates that vue - a JS MVC framework. is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"vue\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"vue\": $VERSION\n\ - \ }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-expressjs-yarn - languages: - - generic - message: "Detected `expressjs` as a dependency in a yarn.lock file. - - This likely indicates that NodeJS web framework Express JS is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: express@$VERSION - - pattern: express@^$VERSION - - pattern: express@~$VERSION - - pattern: express@>$VERSION - - pattern: express@<$VERSION - - pattern: express@>=$VERSION - - pattern: express@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-expressjs - languages: - - json - message: "Detected `expressjs` as a dependency in a package.json file. - - This likely indicates that NodeJS web framework Express JS is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"express\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"express\": $VERSION\n\ - \ }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-jsonwebtoken-yarn - languages: - - generic - message: "Detected `jsonwebtoken` as a dependency in a yarn.lock file. - - This likely indicates that JWT is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: jsonwebtoken@$VERSION - - pattern: jsonwebtoken@^$VERSION - - pattern: jsonwebtoken@~$VERSION - - pattern: jsonwebtoken@>$VERSION - - pattern: jsonwebtoken@<$VERSION - - pattern: jsonwebtoken@>=$VERSION - - pattern: jsonwebtoken@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-jsonwebtoken - languages: - - json - message: "Detected `jsonwebtoken` as a dependency in a package.json file. - - This likely indicates that JWT is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"jsonwebtoken\":\ - \ $VERSION\n }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"jsonwebtoken\"\ - : $VERSION\n }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-pug-yarn - languages: - - generic - message: "Detected `pug` as a dependency in a yarn.lock file. - - This likely indicates that Pug template engine is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: pug@$VERSION - - pattern: pug@^$VERSION - - pattern: pug@~$VERSION - - pattern: pug@>$VERSION - - pattern: pug@<$VERSION - - pattern: pug@>=$VERSION - - pattern: pug@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-pug - languages: - - json - message: "Detected `pug` as a dependency in a package.json file. - - This likely indicates that Pug template engine is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"pug\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"pug\": $VERSION\n\ - \ }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-puppeteer-yarn - languages: - - generic - message: "Detected `puppeteer` as a dependency in a yarn.lock file. - - This likely indicates that headless-browser is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: puppeteer@$VERSION - - pattern: puppeteer@^$VERSION - - pattern: puppeteer@~$VERSION - - pattern: puppeteer@>$VERSION - - pattern: puppeteer@<$VERSION - - pattern: puppeteer@>=$VERSION - - pattern: puppeteer@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-puppeteer - languages: - - json - message: "Detected `puppeteer` as a dependency in a package.json file. - - This likely indicates that headless-browser is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"puppeteer\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"puppeteer\":\ - \ $VERSION\n }\n}\n" - severity: INVENTORY - - id: cai-javascript-dependency-react-yarn - languages: - - generic - message: "Detected `react` as a dependency in a yarn.lock file. - - This likely indicates that React frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - yarn.lock - pattern-either: - - pattern: react@$VERSION - - pattern: react@^$VERSION - - pattern: react@~$VERSION - - pattern: react@>$VERSION - - pattern: react@<$VERSION - - pattern: react@>=$VERSION - - pattern: react@<=$VERSION - severity: INVENTORY - - id: cai-javascript-dependency-react - languages: - - json - message: "Detected `react` as a dependency in a package.json file. - - This likely indicates that React frontend framework is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - package.json - patterns: - - pattern-either: - - pattern: - "{\n ...,\n \"dependencies\": {\n ...,\n \"react\": $VERSION\n\ - \ }\n}\n" - - pattern: - "{\n ...,\n \"devDependencies\": {\n ...,\n \"react\": $VERSION\n\ - \ }\n}\n" - severity: INVENTORY - - id: cai-javascript-framework-expressjs - languages: - - javascript - message: - Detected `express` being required and used, meaning this app uses the ExpressJS - web framework. - metadata: - license: Semgrep Registry License - pattern: 'var $IMPORT = require("express"); - - ... - - var $APP = $IMPORT(); - - ' - severity: INVENTORY - - id: cai-python-dependency-boto - languages: - - generic - message: "Detected `boto` as a dependency in a requirements.txt file. - - This likely indicates that AWS SDK for Python is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - pattern-regex: boto([3|core])* - severity: INVENTORY - - id: cai-python-dependency-django - languages: - - generic - message: "Detected `django` as a dependency in a requirements.txt file. - - This likely indicates that Detected `django` as a dependency in a requirements.txt - file. This likely indicates that the Python web framework Django is being used. - is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - pattern-regex: "[d|D]jango" - severity: INVENTORY - - id: cai-python-dependency-fastapi - languages: - - generic - message: "Detected `fastapi` as a dependency in a requirements.txt file. - - This likely indicates that Python web framework FastAPI is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - pattern-regex: fastapi - severity: INVENTORY - - id: cai-python-dependency-flask - languages: - - generic - message: "Detected `flask` as a dependency in a requirements.txt file. - - This likely indicates that Detected `flask` as a dependency in a requirements.txt - file. This likely indicates that the Python web framework Flask is being used. - is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - patterns: - - pattern-regex: "[Ff]lask" - - pattern-not-regex: "[Ff]lask-" - - pattern-not-regex: -[Ff]lask - severity: INVENTORY - - id: cai-python-dependency-jinja - languages: - - generic - message: "Detected `jinja` as a dependency in a requirements.txt file. - - This likely indicates that Jinja template engine is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - pattern-regex: "[Jj]inja" - severity: INVENTORY - - id: cai-python-dependency-sqlalchemy - languages: - - generic - message: "Detected `sqlalchemy` as a dependency in a requirements.txt file. - - This likely indicates that database is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - requirements.txt - - requirements.in - - setup.py - pattern-regex: "[Ss][Qq][Ll][Aa]lchemy" - severity: INVENTORY - - id: cai-python-framework-django - languages: - - python - message: - Detected `django.http` being imported, indicating this app uses the Python - Django web framework - metadata: - license: Semgrep Registry License - pattern: "import django.http.$FOO - - " - severity: INVENTORY - - id: cai-python-framework-flask - languages: - - python - message: - Detected `Flask()` being instantiated, indicating this app uses the Python - Flask web framework - metadata: - license: Semgrep Registry License - pattern: "$FLASK = Flask(...) - - " - severity: INVENTORY - - id: cai-ocaml-technology-opam - languages: - - generic - message: Detected a OPAM file. This likely indicates OCaml and OPAM are being used. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.opam" - pattern-regex: ^.* - severity: INVENTORY - - id: cai-ocaml-technology-dune - languages: - - generic - message: Detected a dune file. This likely indicates OCaml and dune are being used. - metadata: - license: Semgrep Registry License - paths: - include: - - dune - - dune-project - pattern-regex: ^.* - severity: INVENTORY - - id: cai-ruby-dependency-rails - languages: - - generic - message: "Detected `rails` as a dependency in a Gemfile file. - - This likely indicates that Ruby web framework Rails is being used." - metadata: - license: Semgrep Registry License - paths: - include: - - Gemfile - pattern-regex: gem ['"]rails - severity: INVENTORY - - id: cai-ruby-framework-rails - languages: - - ruby - message: - Detected a rails route being created, indicating this app uses the Ruby - rails web framework - metadata: - license: Semgrep Registry License - pattern: "Rails.application.routes.draw do \n ...\nend\n" - severity: INVENTORY - - id: cai-generic-technology-nginx - languages: - - generic - message: Detected a .conf file. This likely indicates that Nginx is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.conf" - pattern: "location ... {\n ...\n ...\n ...\n}\n" - severity: INVENTORY - - id: cai-hcl-technology-terraform - languages: - - generic - message: Detected a .tf file. This likely indicates that Terraform is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.tf" - pattern: "terraform {\n ...\n ...\n ...\n}\n" - severity: INVENTORY - - id: cai-shell-technology-bash - languages: - - generic - message: Detected a bash script. This likely indicates bash is being used. - metadata: - license: Semgrep Registry License - pattern-either: - - pattern: "#!/bin/bash" - - pattern: "#!/usr/bin/env ... bash" - severity: INVENTORY - - id: cai-shell-technology-sh - languages: - - generic - message: Detected a sh script. This likely indicates sh is being used. - metadata: - license: Semgrep Registry License - pattern-either: - - pattern: "#!/bin/sh" - - pattern: "#!/usr/bin/env ... sh" - severity: INVENTORY - - id: cai-c-technology-c - languages: - - generic - message: Detected a .c file. This likely indicates C is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.c" - pattern-regex: ^. - severity: INVENTORY - - id: cai-csharp-technology-csharp - languages: - - generic - message: Detected a .cs file. This likely indicates C is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.cs" - pattern-regex: ^. - severity: INVENTORY - - id: cai-go-technology-go - languages: - - generic - message: Detected a .go file. This likely indicates Go is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.go" - pattern-regex: ^. - severity: INVENTORY - - id: cai-html-technology-html - languages: - - generic - message: Detected a .html file. This likely indicates html is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.html" - pattern-regex: ^. - severity: INVENTORY - - id: cai-java-technology-java - languages: - - generic - message: Detected a .java file. This likely indicates Java is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.java" - pattern-regex: ^. - severity: INVENTORY - - id: cai-javascript-technology-javascript - languages: - - generic - message: Detected a .js file. This likely indicates JavaScript is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.js" - - "*.javascript" - pattern-regex: ^. - severity: INVENTORY - - id: cai-kotlin-technology-kotlin - languages: - - generic - message: Detected a .kt file. This likely indicates Kotlin is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.kt" - pattern-regex: ^. - severity: INVENTORY - - id: cai-ocaml-technology-ocaml - languages: - - generic - message: Detected a .ml file. This likely indicates Ocaml is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.ml" - pattern-regex: ^. - severity: INVENTORY - - id: cai-php-technology-php - languages: - - generic - message: Detected a .php file. This likely indicates PHP is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.phtml" - - "*.php" - - "*.php3" - - "*.php4" - - "*.php5" - - "*.phps" - pattern-regex: ^. - severity: INVENTORY - - id: cai-python-technology-python - languages: - - generic - message: Detected a .py file. This likely indicates py is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.py" - pattern-regex: ^. - severity: INVENTORY - - id: cai-ruby-technology-ruby - languages: - - generic - message: Detected a .rb file. This likely indicates Ruby is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.rb" - - "*.erb" - pattern-regex: ^. - severity: INVENTORY - - id: cai-scala-technology-scala - languages: - - generic - message: Detected a .scala file. This likely indicates Scala is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.scala" - - "*.sc" - pattern-regex: ^. - severity: INVENTORY - - id: cai-terraform-technology-terraform - languages: - - generic - message: Detected a .tf file. This likely indicates Terraform is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.tf" - pattern-regex: ^. - severity: INVENTORY - - id: cai-typescript-technology-typescript - languages: - - generic - message: Detected a .ts file. This likely indicates TypeScript is in use. - metadata: - license: Semgrep Registry License - paths: - include: - - "*.ts" - - "*.typescript" - pattern-regex: ^. - severity: INVENTORY diff --git a/scripts/run-tests b/scripts/run-tests index 05fc9be54f..c17e5f18d1 100755 --- a/scripts/run-tests +++ b/scripts/run-tests @@ -45,7 +45,7 @@ fi # set_rule_folders() { rule_folders=$(find . -mindepth 1 -maxdepth 1 -type d \ - | grep -v '^./\(\..*\|stats\|trusted_python\|fingerprints\|scripts\|libsonnet\|apex\|elixir\)/\?$' \ + | grep -v '^./\(\..*\|stats\|trusted_python\|scripts\|libsonnet\|apex\|elixir\)/\?$' \ | sort) if [[ -z "$rule_folders" ]]; then error "Cannot find any rule folders to scan in $(pwd)" From 8ce8781dedb9da05ea1f7a59cd1751fc81340d47 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Mon, 23 Sep 2024 09:13:05 +0200 Subject: [PATCH 06/28] chore: Fix some wrong annotations (#3476) test plan: osemgrep test on those dirs do not report any more warnings about wrong annotations --- python/lang/security/audit/conn_recv.py | 2 +- python/lang/security/deserialization/pickle.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/python/lang/security/audit/conn_recv.py b/python/lang/security/audit/conn_recv.py index 1a517ecde1..231dd4ffdd 100644 --- a/python/lang/security/audit/conn_recv.py +++ b/python/lang/security/audit/conn_recv.py @@ -12,5 +12,5 @@ output = {} connection.send(output) -# toodoruleid:multiprocessing.recv +# todoruleid:multiprocessing.recv rx = connection.recv() diff --git a/python/lang/security/deserialization/pickle.py b/python/lang/security/deserialization/pickle.py index 436f34b45b..5a1e8655ec 100644 --- a/python/lang/security/deserialization/pickle.py +++ b/python/lang/security/deserialization/pickle.py @@ -17,9 +17,6 @@ def serialize_exploit(): # Application insecurely deserializes the attacker's serialized data def insecure_deserialization(exploit_code): - # todok: avoid-pickle - # _pickle.loads(exploit_code) - # ruleid: avoid-pickle _pickle.loads(exploit_code) From de1405bcff66fad4c7b270682e88a4c210779b2e Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Mon, 23 Sep 2024 20:46:14 +0200 Subject: [PATCH 07/28] Run osemgrep test --pro on apex/ and elixir/ too (#3478) test plan: wait for green CI checks --- .github/workflows/semgrep-rules-test-develop.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index 50fff01293..911b306cc6 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -19,11 +19,11 @@ jobs: - uses: actions/checkout@v2 with: path: semgrep-rules + - name: run osemgrep test --pro + run: docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep test --pro . + #TODO: we can delete all the rest below and also scripts/run-tests - name: delete directories not containing rules run: rm -rf semgrep-rules/stats - #TODO: in theory we could run those tests by using --test --pro - # since our docker image now contains semgrep-core-pro (but - # it would require to be logged in though via a SEMGREP_APP_TOKEN) - name: delete rules requiring Semgrep Pro run: rm -rf semgrep-rules/apex semgrep-rules/elixir # TODO: this takes 1m20 in CI and could be optimized by switching to osemgrep From 959c8931021a81a6defcf73c63ddf31cc215a1dd Mon Sep 17 00:00:00 2001 From: Martin Jambon Date: Wed, 25 Sep 2024 01:29:49 -0700 Subject: [PATCH 08/28] Named metavariable bug was fixed (#3477) * Named metavariable bug for CMD-like instructions using array syntax was fixed * Update the expected autofixes --- dockerfile/security/missing-user-entrypoint.dockerfile | 3 +-- dockerfile/security/missing-user-entrypoint.fixed.dockerfile | 4 ++-- dockerfile/security/missing-user.dockerfile | 3 +-- dockerfile/security/missing-user.fixed.dockerfile | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/dockerfile/security/missing-user-entrypoint.dockerfile b/dockerfile/security/missing-user-entrypoint.dockerfile index d3110f1814..f7a6c8e86d 100644 --- a/dockerfile/security/missing-user-entrypoint.dockerfile +++ b/dockerfile/security/missing-user-entrypoint.dockerfile @@ -9,6 +9,5 @@ RUN pip3 install semgrep # ruleid: missing-user-entrypoint ENTRYPOINT semgrep -f p/xss -# TODO: metavar bug -# ok: missing-user-entrypoint +# ruleid: missing-user-entrypoint ENTRYPOINT ["semgrep", "--config", "localfile", "targets"] diff --git a/dockerfile/security/missing-user-entrypoint.fixed.dockerfile b/dockerfile/security/missing-user-entrypoint.fixed.dockerfile index 170b6fd3bd..4e5ed36b01 100644 --- a/dockerfile/security/missing-user-entrypoint.fixed.dockerfile +++ b/dockerfile/security/missing-user-entrypoint.fixed.dockerfile @@ -10,6 +10,6 @@ RUN pip3 install semgrep USER non-root ENTRYPOINT semgrep -f p/xss -# TODO: metavar bug -# ok: missing-user-entrypoint +# ruleid: missing-user-entrypoint +USER non-root ENTRYPOINT ["semgrep", "--config", "localfile", "targets"] diff --git a/dockerfile/security/missing-user.dockerfile b/dockerfile/security/missing-user.dockerfile index a089ea0e8d..a5dc45a569 100644 --- a/dockerfile/security/missing-user.dockerfile +++ b/dockerfile/security/missing-user.dockerfile @@ -12,6 +12,5 @@ CMD semgrep -f p/xss # ruleid: missing-user CMD semgrep --config localfile targets -# TODO: metavar ellipses bug -# ok: missing-user +# ruleid: missing-user CMD ["semgrep", "--version"] diff --git a/dockerfile/security/missing-user.fixed.dockerfile b/dockerfile/security/missing-user.fixed.dockerfile index 90c753f54e..28756154d1 100644 --- a/dockerfile/security/missing-user.fixed.dockerfile +++ b/dockerfile/security/missing-user.fixed.dockerfile @@ -14,6 +14,6 @@ CMD semgrep -f p/xss USER non-root CMD semgrep --config localfile targets -# TODO: metavar ellipses bug -# ok: missing-user +# ruleid: missing-user +USER non-root CMD ["semgrep", "--version"] From 81e40c5acfbccba211ca9b8857c7011ac57741da Mon Sep 17 00:00:00 2001 From: Claudio Date: Wed, 25 Sep 2024 22:07:21 +0200 Subject: [PATCH 09/28] Fix test annotation in conn_recv.py (#3479) --- python/lang/security/audit/conn_recv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lang/security/audit/conn_recv.py b/python/lang/security/audit/conn_recv.py index 231dd4ffdd..3de79815e8 100644 --- a/python/lang/security/audit/conn_recv.py +++ b/python/lang/security/audit/conn_recv.py @@ -12,5 +12,5 @@ output = {} connection.send(output) -# todoruleid:multiprocessing.recv +# todoruleid:multiprocessing-recv rx = connection.recv() From ecba02cd37a38d305596d02227edf0d81bb81541 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Thu, 26 Sep 2024 14:21:13 +0200 Subject: [PATCH 10/28] run osemgrep validate --pro (#3481) test plan: osemgrep validate --pro . --- .github/workflows/semgrep-rules-test-develop.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index 911b306cc6..38e6a4ae03 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -19,6 +19,8 @@ jobs: - uses: actions/checkout@v2 with: path: semgrep-rules + - name: run osemgrep validate --pro + run: docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep validate --pro . - name: run osemgrep test --pro run: docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep test --pro . #TODO: we can delete all the rest below and also scripts/run-tests From ed75fb18a676ba036f4015fe59a831252f0ac470 Mon Sep 17 00:00:00 2001 From: Anton Abashkin Date: Sat, 5 Oct 2024 05:22:52 -0400 Subject: [PATCH 11/28] Rule: OpenAI isConsequential flag set to false for state changing operation in OpenAPI spec (#3446) * Rule: OpenAI isConsequential flag set to false for state changing operation in OpenAPI spec * set subcategory to audit instead of vuln * alternative approach --------- Co-authored-by: Pieter De Cremer (Semgrep) --- ...penai-consequential-action-false.test.yaml | 41 ++++++++++++++++ .../openai-consequential-action-false.yaml | 47 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 yaml/openapi/security/openai-consequential-action-false.test.yaml create mode 100644 yaml/openapi/security/openai-consequential-action-false.yaml diff --git a/yaml/openapi/security/openai-consequential-action-false.test.yaml b/yaml/openapi/security/openai-consequential-action-false.test.yaml new file mode 100644 index 0000000000..28eb92ebea --- /dev/null +++ b/yaml/openapi/security/openai-consequential-action-false.test.yaml @@ -0,0 +1,41 @@ +openapi: 3.1.0 +info: + title: Email Service API + version: 1.0.0 + description: API for managing emails +paths: + /emails/{emailId}: + # ok: openai-consequential-action-false + get: + operationId: getEmailById + x-openai-isConsequential: false + summary: List Emails + description: Get a list of email messages. + + # ruleid: openai-consequential-action-false + delete: + operationId: deleteEmailById + x-openai-isConsequential: false + summary: Delete Email + description: Delete a specific email. + + # ruleid: openai-consequential-action-false + post: + operationId: createEmail + x-openai-isConsequential: false + summary: Create Email + description: Create a new email. + + # ruleid: openai-consequential-action-false + put: + operationId: updateEmail + x-openai-isConsequential: false + summary: Update Email + description: Update an existing email. + + # ruleid: openai-consequential-action-false + patch: + operationId: partialUpdateEmail + x-openai-isConsequential: false + summary: Partially Update Email + description: Update certain fields of an existing email. diff --git a/yaml/openapi/security/openai-consequential-action-false.yaml b/yaml/openapi/security/openai-consequential-action-false.yaml new file mode 100644 index 0000000000..d54163e597 --- /dev/null +++ b/yaml/openapi/security/openai-consequential-action-false.yaml @@ -0,0 +1,47 @@ +rules: + - id: openai-consequential-action-false + languages: [yaml] + message: >- + Found 'x-openai-isConsequential: false' in a state-changing HTTP + method: $METHOD $PATH. This Action configuration will enable the 'Always + Allow' option for state-changing HTTP methods, such as POST, PUT, PATCH, + or DELETE. The risk of a user selecting the 'Always Allow' button is that + the agent could perform unintended actions on behalf of the user. When + working with sensitive functionality, it is always best to include a Human + In The Loop (HITL) type of control. Consider the trade-off between security + and user friction and then make a risk-based decision about this function. + severity: WARNING + pattern-either: + - pattern-inside: | + post: + ... + x-openai-isConsequential: false + - pattern-inside: | + put: + ... + x-openai-isConsequential: false + - pattern-inside: | + patch: + ... + x-openai-isConsequential: false + - pattern-inside: | + delete: + ... + x-openai-isConsequential: false + metadata: + category: security + subcategory: + - audit + technology: + - openapi + - openai + likelihood: HIGH + impact: HIGH + confidence: HIGH + owasp: + - 'A04:2021 Insecure Design' + - 'LLM08:2023 - Excessive Agency' + references: + - https://platform.openai.com/docs/actions/consequential-flag + - https://owasp.org/Top10/A04_2021-Insecure_Design/ + - https://owasp.org/www-project-top-10-for-large-language-model-applications/assets/PDF/OWASP-Top-10-for-LLMs-2023-v1_1.pdf From 6364d2b3f3bca81963b995d32ac4aacc8a68bd9f Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Mon, 7 Oct 2024 04:49:35 -0500 Subject: [PATCH 12/28] Include GitHub discussions as user input source. (#3483) Co-authored-by: Vasilii Ermilov --- yaml/github-actions/security/github-script-injection.yaml | 2 ++ .../github-actions/security/run-shell-injection.test.yaml | 8 ++++++++ yaml/github-actions/security/run-shell-injection.yaml | 2 ++ 3 files changed, 12 insertions(+) diff --git a/yaml/github-actions/security/github-script-injection.yaml b/yaml/github-actions/security/github-script-injection.yaml index 3c760d6f33..2c2e9a5a7e 100644 --- a/yaml/github-actions/security/github-script-injection.yaml +++ b/yaml/github-actions/security/github-script-injection.yaml @@ -67,4 +67,6 @@ rules: - pattern: ${{ github.event.pull_request.head.repo.default_branch }} - pattern: ${{ github.head_ref }} - pattern: ${{ github.event.inputs ... }} + - pattern: ${{ github.event.discussion.title }} + - pattern: ${{ github.event.discussion.body }} severity: ERROR diff --git a/yaml/github-actions/security/run-shell-injection.test.yaml b/yaml/github-actions/security/run-shell-injection.test.yaml index cf76cdb549..72577f5619 100644 --- a/yaml/github-actions/security/run-shell-injection.test.yaml +++ b/yaml/github-actions/security/run-shell-injection.test.yaml @@ -121,3 +121,11 @@ jobs: gh api $url > "$name.zip" unzip -d "$name" "$name.zip" done + - name: Show discussion title + # ruleid: run-shell-injection + run: | + echo "${{ github.event.discussion.title }}" + - name: Show discussion body + # ruleid: run-shell-injection + run: | + echo "${{ github.event.discussion.body }}" \ No newline at end of file diff --git a/yaml/github-actions/security/run-shell-injection.yaml b/yaml/github-actions/security/run-shell-injection.yaml index af106986b5..831b10d27c 100644 --- a/yaml/github-actions/security/run-shell-injection.yaml +++ b/yaml/github-actions/security/run-shell-injection.yaml @@ -55,4 +55,6 @@ rules: - pattern: ${{ github.event.pull_request.head.repo.default_branch }} - pattern: ${{ github.head_ref }} - pattern: ${{ github.event.inputs ... }} + - pattern: ${{ github.event.discussion.title }} + - pattern: ${{ github.event.discussion.body }} severity: ERROR From ca4501125ecfcd1eff4d612092a516dc4367e40b Mon Sep 17 00:00:00 2001 From: Dylan Barlett Date: Mon, 7 Oct 2024 06:09:29 -0400 Subject: [PATCH 13/28] Exclude Slack webhook sample URL (#3482) * Exclude Slack webhook sample URL. * Test case for excluding Slack webhook sample URL. --------- Co-authored-by: Vasilii Ermilov --- generic/secrets/security/detected-slack-webhook.txt | 5 ++++- generic/secrets/security/detected-slack-webhook.yaml | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/generic/secrets/security/detected-slack-webhook.txt b/generic/secrets/security/detected-slack-webhook.txt index 6e5fde8401..aa4a91c3ae 100644 --- a/generic/secrets/security/detected-slack-webhook.txt +++ b/generic/secrets/security/detected-slack-webhook.txt @@ -2,4 +2,7 @@ https://hooks.slack.com/services/T12345678/B12345678/abcd1234efgh5678ijkl90zy # ruleid: detected-slack-webhook -https://hooks.slack.com/services/T12345678/B1234567890/abcd1234efgh5678ijkl90zy \ No newline at end of file +https://hooks.slack.com/services/T12345678/B1234567890/abcd1234efgh5678ijkl90zy + +# ok: detected-slack-webhook +https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX diff --git a/generic/secrets/security/detected-slack-webhook.yaml b/generic/secrets/security/detected-slack-webhook.yaml index 51cf6ba6d7..0f35a67a06 100644 --- a/generic/secrets/security/detected-slack-webhook.yaml +++ b/generic/secrets/security/detected-slack-webhook.yaml @@ -1,6 +1,8 @@ rules: - id: detected-slack-webhook - pattern-regex: https://hooks\.slack\.com/services/T[a-zA-Z0-9_]{8,10}/B[a-zA-Z0-9_]{8,10}/[a-zA-Z0-9_]{24} + patterns: + - pattern-regex: https://hooks\.slack\.com/services/T[a-zA-Z0-9_]{8,10}/B[a-zA-Z0-9_]{8,10}/[a-zA-Z0-9_]{24} + - pattern-not: https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX languages: [regex] message: Slack Webhook detected severity: ERROR From 5aa4f20387f6992e95d8183eadedee3d107612ae Mon Sep 17 00:00:00 2001 From: "semgrep-dev-pr-bot[bot]" <63393893+semgrep-dev-pr-bot[bot]@users.noreply.github.com> Date: Mon, 7 Oct 2024 19:39:58 +0700 Subject: [PATCH 14/28] New Published Rules - semgrep.check-is-none-explicitly (#3480) * add semgrep/check-is-none-explicitly.yaml * add semgrep/check-is-none-explicitly.py * move new rule to correctness directory --------- Co-authored-by: Clara McCreery Co-authored-by: Vasilii --- .../correctness/check-is-none-explicitly.py | 19 +++++++++++++++++++ .../correctness/check-is-none-explicitly.yaml | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 python/correctness/check-is-none-explicitly.py create mode 100644 python/correctness/check-is-none-explicitly.yaml diff --git a/python/correctness/check-is-none-explicitly.py b/python/correctness/check-is-none-explicitly.py new file mode 100644 index 0000000000..fa02d4aeda --- /dev/null +++ b/python/correctness/check-is-none-explicitly.py @@ -0,0 +1,19 @@ +# ruleid: check-is-none-explicitly +if record and record == 0: + print("hello, this will never happen") + +# ok: check-is-none-explicitly +if record is not None and record == 0: + print("this is fine") + +# ruleid: check-is-none-explicitly +if record.a and record.a == 0: + print("Not reachable") + +# ruleid: check-is-none-explicitly +if record.a.get("H") and record.a["H"] == 0: + print("Not reachable") + +# ok: check-is-none-explicitly +if record.a.get("I") and record.a["J"] == 0: + print("This is also fine") \ No newline at end of file diff --git a/python/correctness/check-is-none-explicitly.yaml b/python/correctness/check-is-none-explicitly.yaml new file mode 100644 index 0000000000..93163936bb --- /dev/null +++ b/python/correctness/check-is-none-explicitly.yaml @@ -0,0 +1,19 @@ +rules: +- id: check-is-none-explicitly + pattern-either: + - pattern: $X and $X == 0 + - pattern: $X.get($FIELD) and $X[$FIELD] == 0 + fix: ($X != None) and $X == 0 + message: This expression will always return False because 0 is a false-y value. + So if $X is 0, then the first part of this expression will return False but if + it is not, the second part will return False. Perhaps you meant to check if $X + was None explicitly. + languages: + - python + severity: WARNING + metadata: + category: correctness + technology: + - none + references: + - https://www.freecodecamp.org/news/truthy-and-falsy-values-in-python/ From 0da2dcec566011e8fcc1dbf7e76387b3972c2f36 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Mon, 7 Oct 2024 20:06:31 +0200 Subject: [PATCH 15/28] todoruleid: -> proruleid: for solidity test (#3484) test plan: osemgrep test --pro solidity --- solidity/security/delegatecall-to-arbitrary-address.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solidity/security/delegatecall-to-arbitrary-address.sol b/solidity/security/delegatecall-to-arbitrary-address.sol index 176801df95..fe9d30b47a 100644 --- a/solidity/security/delegatecall-to-arbitrary-address.sol +++ b/solidity/security/delegatecall-to-arbitrary-address.sol @@ -35,8 +35,8 @@ contract Test{ } function sink(address _contract, uint256 _num) internal { - // intraprocedural tainting does not work for now... - // todoruleid: delegatecall-to-arbitrary-address + // this requires intraprocedural tainting (--pro-intrafile) + // proruleid: delegatecall-to-arbitrary-address (bool success, bytes memory data) = _contract.delegatecall( abi.encodeWithSignature("setVars(uint256)", _num) ); From b4eb008ed092d2b348a73984f20c72bc6e607ddd Mon Sep 17 00:00:00 2001 From: Alex Leahu Date: Tue, 15 Oct 2024 01:51:59 -0500 Subject: [PATCH 16/28] go: reverseproxy-director and shared-url-struct-mutation (#3486) --- go/lang/security/reverseproxy-director.go | 65 ++++++++++ go/lang/security/reverseproxy-director.yaml | 33 +++++ .../security/shared-url-struct-mutation.go | 118 ++++++++++++++++++ .../security/shared-url-struct-mutation.yaml | 52 ++++++++ 4 files changed, 268 insertions(+) create mode 100644 go/lang/security/reverseproxy-director.go create mode 100644 go/lang/security/reverseproxy-director.yaml create mode 100644 go/lang/security/shared-url-struct-mutation.go create mode 100644 go/lang/security/shared-url-struct-mutation.yaml diff --git a/go/lang/security/reverseproxy-director.go b/go/lang/security/reverseproxy-director.go new file mode 100644 index 0000000000..0a7b1762e1 --- /dev/null +++ b/go/lang/security/reverseproxy-director.go @@ -0,0 +1,65 @@ +package main + +import ( + "log" + "net/http" + "net/http/httputil" + "net/url" +) + +func NewProxy(targetHost string) (*httputil.ReverseProxy, error) { + url, err := url.Parse(targetHost) + if err != nil { + return nil, err + } + + proxy := httputil.NewSingleHostReverseProxy(url) + + originalDirector := proxy.Director + // ruleid: reverseproxy-director + proxy.Director = func(req *http.Request) { + originalDirector(req) + modifyRequest(req) + } + return proxy, nil +} + +func modifyRequest(req *http.Request) { + req.Header.Set("Extra-Header", "nice") +} + +func ProxyRequestHandler(proxy *httputil.ReverseProxy) func(http.ResponseWriter, *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + proxy.ServeHTTP(w, r) + } +} + +type Fake struct { + Director string +} + +func extraCases() { + rp := &httputil.ReverseProxy{ + // ruleid: reverseproxy-director + Director: func(req *http.Request) { + modifyRequest(req) + }, + } + _ = rp + + f := Fake{ + // ok: reverseproxy-director + Director: "abcd", + } + _ = f +} + +func main() { + proxy, err := NewProxy("https://example.com") + if err != nil { + panic(err) + } + + http.HandleFunc("/", ProxyRequestHandler(proxy)) + log.Fatal(http.ListenAndServe(":8080", nil)) +} diff --git a/go/lang/security/reverseproxy-director.yaml b/go/lang/security/reverseproxy-director.yaml new file mode 100644 index 0000000000..ba210b6c72 --- /dev/null +++ b/go/lang/security/reverseproxy-director.yaml @@ -0,0 +1,33 @@ +rules: +- id: reverseproxy-director + message: >- + ReverseProxy can remove headers added by Director. Consider using ReverseProxy.Rewrite + instead of ReverseProxy.Director. + languages: [go] + severity: WARNING + patterns: + - pattern-inside: | + import "net/http/httputil" + ... + - pattern-either: + - pattern: $PROXY.Director = $FUNC + - patterns: + - pattern-inside: | + httputil.ReverseProxy{ + ... + } + - pattern: | + Director: $FUNC + metadata: + cwe: + - "CWE-115: Misinterpretation of Input" + category: security + subcategory: + - audit + technology: + - go + confidence: MEDIUM + likelihood: LOW + impact: LOW + references: + - https://github.com/golang/go/issues/50580 diff --git a/go/lang/security/shared-url-struct-mutation.go b/go/lang/security/shared-url-struct-mutation.go new file mode 100644 index 0000000000..005d20075b --- /dev/null +++ b/go/lang/security/shared-url-struct-mutation.go @@ -0,0 +1,118 @@ +package main + +import ( + "net/http" + "net/url" +) + +var redirectURL, _ = url.Parse("https://example.com") + +func getRedirectToken() (string, error) { + return "abcd", nil +} + +func handler1(w http.ResponseWriter, r *http.Request) { + u := redirectURL + q := u.Query() + + // opaque process that might fail + token, err := getRedirectToken() + if err != nil { + q.Set("error", err.Error()) + } else { + q.Set("token", token) + } + // ruleid: shared-url-struct-mutation + u.RawQuery = q.Encode() + r.URL.RawQuery = q.Encode() + + http.Redirect(w, r, u.String(), http.StatusFound) +} + +func handler2(w http.ResponseWriter, r *http.Request) { + u, _ := url.Parse("https://example.com") + + q := u.Query() + + // opaque process that might fail + token, err := getRedirectToken() + if err != nil { + q.Set("error", err.Error()) + } else { + q.Set("token", token) + } + // ok: shared-url-struct-mutation + u.RawQuery = q.Encode() + + http.Redirect(w, r, u.String(), http.StatusFound) +} + +func handler3(w http.ResponseWriter, r *http.Request) { + u := url.URL{ + Scheme: "https", + Host: "example.com", + Path: "/", + } + q := u.Query() + + // opaque process that might fail + token, err := getRedirectToken() + if err != nil { + q.Set("error", err.Error()) + } else { + q.Set("token", token) + } + + u.RawQuery = q.Encode() + + http.Redirect(w, r, u.String(), http.StatusFound) +} + +func handler4(w http.ResponseWriter, r *http.Request) { + var u *url.URL + if true { + u, _ = url.Parse("https://example.com") + } + + if u != nil { + + q := u.Query() + + // opaque process that might fail + token, err := getRedirectToken() + if err != nil { + q.Set("error", err.Error()) + } else { + q.Set("token", token) + } + // ok: shared-url-struct-mutation + u.RawQuery = q.Encode() + + http.Redirect(w, r, u.String(), http.StatusFound) + } + http.Redirect(w, r, "https://google.com", http.StatusFound) +} + +func extraCases(w http.ResponseWriter, r *http.Request) { + var x struct { + y []struct { + Path string + } + } + // ok: shared-url-struct-mutation + r.URL.RawQuery = "abcd" + // ok: shared-url-struct-mutation + x.y[0].Path = "abcd" + + a, _ := url.ParseRequestURI("https://example.com") + // ok: shared-url-struct-mutation + a.RawQuery = "abcd" +} + +func main() { + http.HandleFunc("/1", handler1) + http.HandleFunc("/2", handler2) + http.HandleFunc("/3", handler3) + http.HandleFunc("/4", handler4) + http.ListenAndServe(":7777", nil) +} diff --git a/go/lang/security/shared-url-struct-mutation.yaml b/go/lang/security/shared-url-struct-mutation.yaml new file mode 100644 index 0000000000..0dcd483a15 --- /dev/null +++ b/go/lang/security/shared-url-struct-mutation.yaml @@ -0,0 +1,52 @@ +rules: +- id: shared-url-struct-mutation + message: >- + Shared URL struct may have been accidentally mutated. Ensure that + this behavior is intended. + languages: [go] + severity: WARNING + patterns: + - pattern-inside: | + import "net/url" + ... + - pattern-not-inside: | + ... = url.Parse(...) + ... + - pattern-not-inside: | + ... = url.ParseRequestURI(...) + ... + - pattern-not-inside: | + ... = url.URL{...} + ... + - pattern-not-inside: | + var $URL *$X.URL + ... + - pattern-either: + - pattern: $URL.RawQuery = ... + - pattern: $URL.Path = ... + - pattern: $URL.RawPath = ... + - pattern: $URL.Fragment = ... + - pattern: $URL.RawFragment = ... + - pattern: $URL.Scheme = ... + - pattern: $URL.Opaque = ... + - pattern: $URL.Host = ... + - pattern: $URL.User = ... + - metavariable-pattern: + metavariable: $URL + patterns: + - pattern-not: $X.$Y + - pattern-not: $X[...] + metadata: + cwe: + - "CWE-436: Interpretation Conflict" + category: security + subcategory: + - audit + technology: + - go + confidence: LOW + likelihood: LOW + impact: LOW + references: + - https://github.com/golang/go/issues/63777 + From c865e0cadd83619079d78c500905903c2839aa2c Mon Sep 17 00:00:00 2001 From: Starkteetje Date: Tue, 15 Oct 2024 10:06:52 +0200 Subject: [PATCH 17/28] Add rules around Node.js crypto module (#3357) Co-authored-by: Pieter De Cremer (Semgrep) --- .../node-crypto/security/aead-no-final.js | 114 ++++++++++++++++++ .../node-crypto/security/aead-no-final.yaml | 37 ++++++ .../security/create-de-cipher-no-iv.js | 53 ++++++++ .../security/create-de-cipher-no-iv.yaml | 31 +++++ .../node-crypto/security/gcm-no-tag-length.js | 84 +++++++++++++ .../security/gcm-no-tag-length.yaml | 33 +++++ 6 files changed, 352 insertions(+) create mode 100644 javascript/node-crypto/security/aead-no-final.js create mode 100644 javascript/node-crypto/security/aead-no-final.yaml create mode 100644 javascript/node-crypto/security/create-de-cipher-no-iv.js create mode 100644 javascript/node-crypto/security/create-de-cipher-no-iv.yaml create mode 100644 javascript/node-crypto/security/gcm-no-tag-length.js create mode 100644 javascript/node-crypto/security/gcm-no-tag-length.yaml diff --git a/javascript/node-crypto/security/aead-no-final.js b/javascript/node-crypto/security/aead-no-final.js new file mode 100644 index 0000000000..621ef2b6e2 --- /dev/null +++ b/javascript/node-crypto/security/aead-no-final.js @@ -0,0 +1,114 @@ +const crypto = require('node:crypto') +function decrypt1(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + + // ruleid: aead-no-final + const decipher = crypto.createDecipheriv("aes-128-gcm", key, iv); + let result = decipher.update(encryptedData); + + return result.toString("utf8"); +} + +algo = "aes-192-gcm" +function decrypt2(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ruleid: aead-no-final + var decipher = crypto.createDecipheriv(algo, key, iv); + decipher.setAuthTag(auth); + let result = decipher.update(encryptedData); + + return result.toString("utf8"); +} + +function decrypt3(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ruleid: aead-no-final + const decipher = crypto.createDecipheriv("aes-256-ccm", key, iv, {authTagLength: 16}) + decipher.setAuthTag(auth) + decipher.setAAD(Buffer.alloc(0), {plaintextLength: encryptedData.length}) + let result = decipher.update(encryptedData) + + return result; +} + +function decrypt4(key, ciphertext) { + let encrypted = Buffer.from(ciphertext, 'base64'); + let iv = encrypted.slice(encrypted.byteLength - 12, encrypted.byteLength); + + // ruleid: aead-no-final + let decipher = crypto.createDecipheriv("aes-256-ocb", key, iv, {authTagLength: 16}); + let decrypted = decipher.update(encrypted.slice(0, encrypted.byteLength - 16 - 12)); + + decrypted = Buffer.concat([decrypted]); + return decrypted; +} + +const { createDecipheriv } = require('node:crypto') +function decrypt5(key, ciphertext) { + let iv = ciphertext.slice(ciphertext.byteLength - 12, ciphertext.byteLength); + + // ruleid: aead-no-final + let decipher = createDecipheriv("chacha20-poly1305", key, iv); + let decrypted = decipher.update(ciphertext.slice(0, ciphertext.byteLength - 16 - 12)); + + decrypted = Buffer.concat([decrypted]); + return decrypted; +} + +function decrypt6(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ok: aead-no-final + var decipher = crypto.createDecipheriv("aes-128-gcm", key, iv); + decipher.setAuthTag(auth); + let result = Buffer.concat([decipher.update(encryptedData), decipher.final()]); + + return result; +} + +function decrypt7(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ok: aead-no-final + var decipher = crypto.createDecipheriv("aes-192-ccm", key, iv, {authTagLength: 16}) + decipher.setAuthTag(auth) + let result = decipher.update(encryptedData) + result += decipher.final() + + return result +} + +function decrypt8(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ok: aead-no-final + var decipher = crypto.createDecipheriv("aes-256-ocb", key, iv, {authTagLength: 16}); + decipher.setAuthTag(auth); + return decipher.update(encryptedData) + decipher.final(); +} + +function decrypt9(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // missing final, but not AEAD mode + // ok: aead-no-final + var decipher = crypto.createDecipheriv("aes-192-cbc", key, iv); + let result = decipher.update(encryptedData); + + return result.toString("utf8"); +} diff --git a/javascript/node-crypto/security/aead-no-final.yaml b/javascript/node-crypto/security/aead-no-final.yaml new file mode 100644 index 0000000000..c347553ccc --- /dev/null +++ b/javascript/node-crypto/security/aead-no-final.yaml @@ -0,0 +1,37 @@ +rules: +- id: aead-no-final + message: >- + The 'final' call of a Decipher object checks the authentication tag in a mode for authenticated encryption. + Failing to call 'final' will invalidate all integrity guarantees of the released ciphertext. + metadata: + cwe: + - 'CWE-310: CWE CATEGORY: Cryptographic Issues' + owasp: + - A02:2021 - Cryptographic Failures + category: security + subcategory: + - vuln + technology: + - node-crypto + likelihood: HIGH + impact: MEDIUM + confidence: HIGH + references: + - https://nodejs.org/api/crypto.html#deciphersetauthtagbuffer-encoding + - https://owasp.org/Top10/A02_2021-Cryptographic_Failures/ + languages: + - javascript + - typescript + severity: ERROR + patterns: + - pattern: | + $DECIPHER = $CRYPTO.createDecipheriv('$ALGO', ...) + ... + $DECIPHER.update(...) + - pattern-not-inside: | + $DECIPHER = $CRYPTO.createDecipheriv('$ALGO', ...) + ... + $DECIPHER.final(...) + - metavariable-regex: + metavariable: $ALGO + regex: ".*(-gcm|-ccm|-ocb|chacha20-poly1305)$" diff --git a/javascript/node-crypto/security/create-de-cipher-no-iv.js b/javascript/node-crypto/security/create-de-cipher-no-iv.js new file mode 100644 index 0000000000..e45a53fc44 --- /dev/null +++ b/javascript/node-crypto/security/create-de-cipher-no-iv.js @@ -0,0 +1,53 @@ +const crypto = require('node:crypto') +function decrypt1(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + + // ruleid: create-de-cipher-no-iv + const decipher = crypto.createDecipher("aes-128-gcm", key, iv); + let result = decipher.update(encryptedData); + + return result.toString("utf8"); +} + +function decrypt2(ciphertext, key) { + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ruleid: create-de-cipher-no-iv + var decipher = crypto.createDecipher("aes-192-cbc", key); + let result = decipher.update(encryptedData); + + return result.toString("utf8"); +} + +const { createCipher } = require('node:crypto') +function encrypt1(plaintext, key) { + // ruleid: create-de-cipher-no-iv + const cipher = createCipher("aes-256-ccm", key, {authTagLength: 16}) + cipher.setAAD(Buffer.alloc(0), {plaintextLength: plaintext.length}) + let result = cipher.update(plaintext) + cipher.final() + + return result + cipher.getAuthTag() +} + +function decrypt3(key, ciphertext) { + let encrypted = Buffer.from(ciphertext, 'base64'); + let iv = encrypted.slice(encrypted.byteLength - 12, encrypted.byteLength); + + // ok: create-de-cipher-no-iv + let decipher = crypto.createDecipheriv("aes-256-ocb", key, iv, {authTagLength: 16}); + let decrypted = decipher.update(encrypted.slice(0, encrypted.byteLength - 16 - 12)); + + decrypted = Buffer.concat([decrypted]); + return decrypted; +} + +function encrypt2(key, plaintext) { + let iv = crypto.randomBytes(12); + + // ok: create-de-cipher-no-iv + let cipher = crypto.createCipheriv("chacha20-poly1305", key, iv); + let encrypted = Buffer.concat([cipher.update(plaintext), cipher.final(), cipher.getAuthTag()]); + return encrypted; +} diff --git a/javascript/node-crypto/security/create-de-cipher-no-iv.yaml b/javascript/node-crypto/security/create-de-cipher-no-iv.yaml new file mode 100644 index 0000000000..3cd40e1575 --- /dev/null +++ b/javascript/node-crypto/security/create-de-cipher-no-iv.yaml @@ -0,0 +1,31 @@ +rules: +- id: create-de-cipher-no-iv + message: >- + The deprecated functions 'createCipher' and 'createDecipher' generate the same initialization vector every time. + For counter modes such as CTR, GCM, or CCM this leads to break of both confidentiality and integrity, if the key is used more than once. + Other modes are still affected in their strength, though they're not completely broken. + Use 'createCipheriv' or 'createDecipheriv' instead. + metadata: + cwe: + - 'CWE-1204: Generation of Weak Initialization Vector (IV)' + category: security + subcategory: + - vuln + technology: + - node-crypto + likelihood: HIGH + impact: MEDIUM + confidence: HIGH + references: + - https://nodejs.org/api/crypto.html#cryptocreatecipheralgorithm-password-options + - https://nodejs.org/api/crypto.html#cryptocreatedecipheralgorithm-password-options + languages: + - javascript + - typescript + severity: ERROR + patterns: + - pattern-either: + - pattern: | + $CRYPTO.createCipher(...) + - pattern: | + $CRYPTO.createDecipher(...) diff --git a/javascript/node-crypto/security/gcm-no-tag-length.js b/javascript/node-crypto/security/gcm-no-tag-length.js new file mode 100644 index 0000000000..3c0cfa761e --- /dev/null +++ b/javascript/node-crypto/security/gcm-no-tag-length.js @@ -0,0 +1,84 @@ +const crypto = require('node:crypto') +function decrypt1(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + algo = "aes-128-gcm" + + // ruleid: gcm-no-tag-length + const decipher = crypto.createDecipheriv(algo, key, iv); + decipher.setAuthTag(auth); + let result = decipher.update(encryptedData) + decipher.final(); + + return result.toString("utf8"); +} + +function decrypt2(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + // While this prevents the attack, I'm not quite sure how to capture such length checks and exclude them from the findings + assert(auth.length === 16) + + // ruleid: gcm-no-tag-length + const decipher = crypto.createDecipheriv("aes-192-gcm", key, iv); + decipher.setAuthTag(auth); + let result = decipher.update(encryptedData) + decipher.final(); + + return result.toString("utf8"); +} + +function decrypt3(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ok: gcm-no-tag-length + var decipher = crypto.createDecipheriv("aes-128-gcm", key, iv, {authTagLength: 16}); + decipher.setAuthTag(auth); + let result = Buffer.concat([decipher.update(encryptedData), decipher.final()]); + + return result; +} + +function decrypt4(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // even though auth tag is shorter than it should be, this looks like a conscious choice + // ok: gcm-no-tag-length + var decipher = crypto.createDecipheriv("aes-256-gcm", key, iv, {authTagLength: 4}); + decipher.setAuthTag(auth); + let result = Buffer.concat([decipher.update(encryptedData), decipher.final()]); + + return result; +} + +function decrypt5(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // hard to capture whether options contain 'authTagLength', so to reduce false positives, just ignore any call with options + // ok: gcm-no-tag-length + var decipher = crypto.createDecipheriv("aes-256-gcm", key, iv, {someOption: someValue}) + decipher.setAuthTag(auth) + let result = decipher.update(encryptedData)+ decipher.final() + + return result; +} + +function decrypt6(ciphertext, key) { + iv = ciphertext.iv + encryptedData = ciphertext.data + auth = ciphertext.auth + + // ok: gcm-no-tag-length + var decipher = crypto.createDecipheriv("chacha20-poly1305", key, iv) + decipher.setAuthTag(auth) + let result = decipher.update(encryptedData)+ decipher.final() + + return result; +} diff --git a/javascript/node-crypto/security/gcm-no-tag-length.yaml b/javascript/node-crypto/security/gcm-no-tag-length.yaml new file mode 100644 index 0000000000..0cfc9bb411 --- /dev/null +++ b/javascript/node-crypto/security/gcm-no-tag-length.yaml @@ -0,0 +1,33 @@ +rules: +- id: gcm-no-tag-length + message: >- + The call to 'createDecipheriv' with the Galois Counter Mode (GCM) mode of operation is missing an expected authentication tag length. + If the expected authentication tag length is not specified or otherwise checked, the application might be tricked into verifying a shorter-than-expected authentication tag. + This can be abused by an attacker to spoof ciphertexts or recover the implicit authentication key of GCM, allowing arbitrary forgeries. + metadata: + cwe: + - 'CWE-310: CWE CATEGORY: Cryptographic Issues' + owasp: + - A02:2021 - Cryptographic Failures + category: security + subcategory: + - vuln + technology: + - node-crypto + likelihood: MEDIUM + impact: MEDIUM + confidence: MEDIUM + references: + - https://www.securesystems.de/blog/forging_ciphertexts_under_Galois_Counter_Mode_for_the_Node_js_crypto_module/ + - https://nodejs.org/api/crypto.html#cryptocreatedecipherivalgorithm-key-iv-options + - https://owasp.org/Top10/A02_2021-Cryptographic_Failures/ + languages: + - javascript + - typescript + severity: ERROR + patterns: + - pattern: | + $CRYPTO.createDecipheriv('$ALGO', $KEY, $IV) + - metavariable-regex: + metavariable: $ALGO + regex: ".*(-gcm)$" From 153588f28b9662a005747a81733201c6e39b3e10 Mon Sep 17 00:00:00 2001 From: "Pieter De Cremer (Semgrep)" Date: Tue, 15 Oct 2024 13:40:40 +0200 Subject: [PATCH 18/28] add sanitizer and update message of dangerous subprocess rule (#3487) --- .../audit/dangerous-subprocess-use-tainted-env-args.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/lang/security/audit/dangerous-subprocess-use-tainted-env-args.yaml b/python/lang/security/audit/dangerous-subprocess-use-tainted-env-args.yaml index 199e244057..c7b58315a8 100644 --- a/python/lang/security/audit/dangerous-subprocess-use-tainted-env-args.yaml +++ b/python/lang/security/audit/dangerous-subprocess-use-tainted-env-args.yaml @@ -3,6 +3,8 @@ rules: mode: taint options: symbolic_propagation: true + pattern-sanitizers: + - pattern: shlex.quote(...) pattern-sources: - patterns: - pattern-either: @@ -81,7 +83,7 @@ rules: message: >- Detected subprocess function '$FUNC' with user controlled data. A malicious actor could leverage this to perform command injection. - You may consider using 'shlex.escape()'. + You may consider using 'shlex.quote()'. metadata: owasp: - A01:2017 - Injection From cd697bcf65ec137d9de052e8903a0e32d63ced23 Mon Sep 17 00:00:00 2001 From: "Pieter De Cremer (Semgrep)" Date: Thu, 17 Oct 2024 11:17:33 +0200 Subject: [PATCH 19/28] fix workflow condition being interpreted as a string (#3489) --- .github/workflows/trigger-semgrep-scanner-initiate-scan.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml b/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml index 0c29a8c1fe..c2449ab91b 100644 --- a/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml +++ b/.github/workflows/trigger-semgrep-scanner-initiate-scan.yaml @@ -52,8 +52,6 @@ jobs: HEAD_REF: ${{ github.head_ref }} REPO_NAME: ${{ github.event.repository.name }} PR_HEAD_SHA: ${{github.event.pull_request.head.sha}} - if: | - github.event_name == 'pull_request' && - env.changed_lang_count > 0 + if: github.event_name == 'pull_request' && env.changed_lang_count > 0 run: | curl -X POST https://argoworkflows-dev2.corp.r2c.dev/api/v1/events/security-research/initiate-scan-argo -H "Authorization: ${{ secrets.ARGO_WORKFLOWS_TOKEN }}" -d "{\"branch\" : \"$HEAD_REF\", \"repo\" : \"$REPO_NAME\", \"commit\" : \"$PR_HEAD_SHA\", \"changed_files\" : \"$CHANGED_FILES\" , \"langs\" : \"$CHANGED_LANGS\"}" From 5583c923518d93e95c4170aace7e49d0bfcf4ab3 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Fri, 18 Oct 2024 10:21:08 +0200 Subject: [PATCH 20/28] Fix for osemgrep test --pro with DeepScan too (often deepruleid: -> deeptodoruleid:) (#3490) * Fix for osemgrep test --pro with DeepScan too Mostly some deepruleid: -> deeptodoruleid: as unfortunately the engine is still not good enough to find them test plan: osemgrep-pro test --pro . * fix --- go/lang/security/filepath-clean-misuse.fixed.go | 4 ++-- go/lang/security/filepath-clean-misuse.go | 4 ++-- java/lang/security/audit/tainted-cmd-from-http-request.java | 4 ++-- javascript/browser/security/raw-html-concat.js | 2 +- .../security/insecure-cipher-algorithm-blowfish.py | 4 ++-- python/pycryptodome/security/insecure-cipher-algorithm-des.py | 4 ++-- python/pycryptodome/security/insecure-cipher-algorithm-rc2.py | 4 ++-- python/requests/security/no-auth-over-http.py | 2 +- python/sqlalchemy/security/sqlalchemy-execute-raw-query.py | 2 +- ruby/rails/security/brakeman/check-sql.rb | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/go/lang/security/filepath-clean-misuse.fixed.go b/go/lang/security/filepath-clean-misuse.fixed.go index b36e5e1ddd..9f7ca43d80 100644 --- a/go/lang/security/filepath-clean-misuse.fixed.go +++ b/go/lang/security/filepath-clean-misuse.fixed.go @@ -88,7 +88,7 @@ func main() { // urlPath = "/" + urlPath // r.URL.Path = urlPath // } -// // ok: filepath-clean-misuse +// // ok filepath-clean-misuse // _, err := root.Open(path.Clean(urlPath)) // if err != nil && os.IsNotExist(err) { // r.URL.Path = defaultPath @@ -100,4 +100,4 @@ func main() { // } // handler.ServeHTTP(w, r) // }) -// } \ No newline at end of file +// } diff --git a/go/lang/security/filepath-clean-misuse.go b/go/lang/security/filepath-clean-misuse.go index 52c7611df9..ca33a47e9c 100644 --- a/go/lang/security/filepath-clean-misuse.go +++ b/go/lang/security/filepath-clean-misuse.go @@ -88,7 +88,7 @@ func main() { // urlPath = "/" + urlPath // r.URL.Path = urlPath // } -// // ok: filepath-clean-misuse +// // ok filepath-clean-misuse // _, err := root.Open(path.Clean(urlPath)) // if err != nil && os.IsNotExist(err) { // r.URL.Path = defaultPath @@ -100,4 +100,4 @@ func main() { // } // handler.ServeHTTP(w, r) // }) -// } \ No newline at end of file +// } diff --git a/java/lang/security/audit/tainted-cmd-from-http-request.java b/java/lang/security/audit/tainted-cmd-from-http-request.java index 9cb761fcda..92f99fb0a0 100644 --- a/java/lang/security/audit/tainted-cmd-from-http-request.java +++ b/java/lang/security/audit/tainted-cmd-from-http-request.java @@ -259,11 +259,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) // ruleid: tainted-cmd-from-http-request argList.add("echo " + bar); - // deepruleid: tainted-cmd-from-http-request + // deeptodoruleid: tainted-cmd-from-http-request ProcessBuilder pb = new ProcessBuilder(argList); try { - // deepruleid: tainted-cmd-from-http-request + // deeptodoruleid: tainted-cmd-from-http-request Process p = pb.start(); org.owasp.benchmark.helpers.Utils.printOSCommandResults(p, response); } catch (IOException e) { diff --git a/javascript/browser/security/raw-html-concat.js b/javascript/browser/security/raw-html-concat.js index df342002b4..e4803a9918 100644 --- a/javascript/browser/security/raw-html-concat.js +++ b/javascript/browser/security/raw-html-concat.js @@ -14,7 +14,7 @@ $(function ($) { var x = `
${content}
` - // ruleid: deepok: raw-html-concat + // ruleid: deeptodook: raw-html-concat return '
' + newContent + '
'; }, isInline: false diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py b/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py index b366bec50b..d6706405cf 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py @@ -25,12 +25,12 @@ bs = pycrypto_blowfish.block_size # ruleid:insecure-cipher-algorithm-blowfish cipher = pycrypto_blowfish.new(key, pycrypto_blowfish.MODE_CBC, iv) -# deepruleid:insecure-cipher-algorithm-blowfish +# deeptodoruleid:insecure-cipher-algorithm-blowfish msg = iv + cipher.encrypt(plaintext + padding) bs = pycryptodomex_blowfish.block_size # ruleid:insecure-cipher-algorithm-blowfish cipher = pycryptodomex_blowfish.new(key, pycryptodomex_blowfish.MODE_CBC, iv) -# deepruleid:insecure-cipher-algorithm-blowfish +# deeptodoruleid:insecure-cipher-algorithm-blowfish msg = iv + cipher.encrypt(plaintext + padding) key = b'Sixteen byte key' diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-des.py b/python/pycryptodome/security/insecure-cipher-algorithm-des.py index bc25f6daac..0455ab5592 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-des.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-des.py @@ -23,13 +23,13 @@ ctr = Counter.new(pycrypto_des.block_size*8/2, prefix=nonce) # ruleid:insecure-cipher-algorithm-des cipher = pycrypto_des.new(key, pycrypto_des.MODE_CTR, counter=ctr) -# deepruleid:insecure-cipher-algorithm-des +# deeptodoruleid:insecure-cipher-algorithm-des msg = nonce + cipher.encrypt(plaintext) nonce = Random.new().read(pycryptodomex_des.block_size/2) ctr = Counter.new(pycryptodomex_des.block_size*8/2, prefix=nonce) # ruleid:insecure-cipher-algorithm-des cipher = pycryptodomex_des.new(key, pycryptodomex_des.MODE_CTR, counter=ctr) -# deepruleid:insecure-cipher-algorithm-des +# deeptodoruleid:insecure-cipher-algorithm-des msg = nonce + cipher.encrypt(plaintext) diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py b/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py index 53d7d684c0..145fc958b9 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py @@ -19,11 +19,11 @@ iv = Random.new().read(pycrypto_arc2.block_size) # ruleid:insecure-cipher-algorithm-rc2 cipher = pycrypto_arc2.new(key, pycrypto_arc2.MODE_CFB, iv) -# deepruleid:insecure-cipher-algorithm-rc2 +# deeptodoruleid:insecure-cipher-algorithm-rc2 msg = iv + cipher.encrypt(b'Attack at dawn') # ruleid:insecure-cipher-algorithm-rc2 cipher = pycryptodomex_arc2.new(key, pycryptodomex_arc2.MODE_CFB, iv) -# deepruleid:insecure-cipher-algorithm-rc2 +# deeptodoruleid:insecure-cipher-algorithm-rc2 msg = iv + cipher.encrypt(b'Attack at dawn') key = b'Sixteen byte key' diff --git a/python/requests/security/no-auth-over-http.py b/python/requests/security/no-auth-over-http.py index 3977eb0283..4df3b21edf 100644 --- a/python/requests/security/no-auth-over-http.py +++ b/python/requests/security/no-auth-over-http.py @@ -2,7 +2,7 @@ # ok:no-auth-over-http good_url = "https://www.github.com" -# deepruleid:no-auth-over-http +# deeptodoruleid:no-auth-over-http bad_url = "http://www.github.com" # ruleid:no-auth-over-http diff --git a/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py b/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py index 7b25aacad2..71f1d38c57 100644 --- a/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py +++ b/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py @@ -198,7 +198,7 @@ # ok: sqlalchemy-execute-raw-query engine = create_engine('postgresql://user@localhost/database') query = select(literal_column("users.fullname", String) + ', ' + literal_column("addresses.email_address").label("title")).where(and_(literal_column("users.id") == literal_column("addresses.user_id"), text("users.name BETWEEN 'm' AND 'z'"), text("(addresses.email_address LIKE :x OR addresses.email_address LIKE :y)"))).select_from(table('users')).select_from(table('addresses')) -# deepruleid: sqlalchemy-execute-raw-query +# deeptodoruleid: sqlalchemy-execute-raw-query conn.execute(query, {"x":"%@aol.com", "y":"%@msn.com"}).fetchall() diff --git a/ruby/rails/security/brakeman/check-sql.rb b/ruby/rails/security/brakeman/check-sql.rb index 65e8e8c94f..e6554f36cd 100644 --- a/ruby/rails/security/brakeman/check-sql.rb +++ b/ruby/rails/security/brakeman/check-sql.rb @@ -147,7 +147,7 @@ def test_more_if_statements "blah" end - # ruleid: deepok: check-sql + # ruleid: deeptodook: check-sql Product.last("blah = '#{x}'") #ok: check-sql From 966d1ba1c9585e6bc2099bf5b02bbe04eefe4be4 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Fri, 18 Oct 2024 12:25:59 +0200 Subject: [PATCH 21/28] Revert "Fix for osemgrep test --pro with DeepScan too (often deepruleid: -> deeptodoruleid:) (#3490)" (#3491) This reverts commit 5583c923518d93e95c4170aace7e49d0bfcf4ab3. --- go/lang/security/filepath-clean-misuse.fixed.go | 4 ++-- go/lang/security/filepath-clean-misuse.go | 4 ++-- java/lang/security/audit/tainted-cmd-from-http-request.java | 4 ++-- javascript/browser/security/raw-html-concat.js | 2 +- .../security/insecure-cipher-algorithm-blowfish.py | 4 ++-- python/pycryptodome/security/insecure-cipher-algorithm-des.py | 4 ++-- python/pycryptodome/security/insecure-cipher-algorithm-rc2.py | 4 ++-- python/requests/security/no-auth-over-http.py | 2 +- python/sqlalchemy/security/sqlalchemy-execute-raw-query.py | 2 +- ruby/rails/security/brakeman/check-sql.rb | 2 +- 10 files changed, 16 insertions(+), 16 deletions(-) diff --git a/go/lang/security/filepath-clean-misuse.fixed.go b/go/lang/security/filepath-clean-misuse.fixed.go index 9f7ca43d80..b36e5e1ddd 100644 --- a/go/lang/security/filepath-clean-misuse.fixed.go +++ b/go/lang/security/filepath-clean-misuse.fixed.go @@ -88,7 +88,7 @@ func main() { // urlPath = "/" + urlPath // r.URL.Path = urlPath // } -// // ok filepath-clean-misuse +// // ok: filepath-clean-misuse // _, err := root.Open(path.Clean(urlPath)) // if err != nil && os.IsNotExist(err) { // r.URL.Path = defaultPath @@ -100,4 +100,4 @@ func main() { // } // handler.ServeHTTP(w, r) // }) -// } +// } \ No newline at end of file diff --git a/go/lang/security/filepath-clean-misuse.go b/go/lang/security/filepath-clean-misuse.go index ca33a47e9c..52c7611df9 100644 --- a/go/lang/security/filepath-clean-misuse.go +++ b/go/lang/security/filepath-clean-misuse.go @@ -88,7 +88,7 @@ func main() { // urlPath = "/" + urlPath // r.URL.Path = urlPath // } -// // ok filepath-clean-misuse +// // ok: filepath-clean-misuse // _, err := root.Open(path.Clean(urlPath)) // if err != nil && os.IsNotExist(err) { // r.URL.Path = defaultPath @@ -100,4 +100,4 @@ func main() { // } // handler.ServeHTTP(w, r) // }) -// } +// } \ No newline at end of file diff --git a/java/lang/security/audit/tainted-cmd-from-http-request.java b/java/lang/security/audit/tainted-cmd-from-http-request.java index 92f99fb0a0..9cb761fcda 100644 --- a/java/lang/security/audit/tainted-cmd-from-http-request.java +++ b/java/lang/security/audit/tainted-cmd-from-http-request.java @@ -259,11 +259,11 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) // ruleid: tainted-cmd-from-http-request argList.add("echo " + bar); - // deeptodoruleid: tainted-cmd-from-http-request + // deepruleid: tainted-cmd-from-http-request ProcessBuilder pb = new ProcessBuilder(argList); try { - // deeptodoruleid: tainted-cmd-from-http-request + // deepruleid: tainted-cmd-from-http-request Process p = pb.start(); org.owasp.benchmark.helpers.Utils.printOSCommandResults(p, response); } catch (IOException e) { diff --git a/javascript/browser/security/raw-html-concat.js b/javascript/browser/security/raw-html-concat.js index e4803a9918..df342002b4 100644 --- a/javascript/browser/security/raw-html-concat.js +++ b/javascript/browser/security/raw-html-concat.js @@ -14,7 +14,7 @@ $(function ($) { var x = `
${content}
` - // ruleid: deeptodook: raw-html-concat + // ruleid: deepok: raw-html-concat return '
' + newContent + '
'; }, isInline: false diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py b/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py index d6706405cf..b366bec50b 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-blowfish.py @@ -25,12 +25,12 @@ bs = pycrypto_blowfish.block_size # ruleid:insecure-cipher-algorithm-blowfish cipher = pycrypto_blowfish.new(key, pycrypto_blowfish.MODE_CBC, iv) -# deeptodoruleid:insecure-cipher-algorithm-blowfish +# deepruleid:insecure-cipher-algorithm-blowfish msg = iv + cipher.encrypt(plaintext + padding) bs = pycryptodomex_blowfish.block_size # ruleid:insecure-cipher-algorithm-blowfish cipher = pycryptodomex_blowfish.new(key, pycryptodomex_blowfish.MODE_CBC, iv) -# deeptodoruleid:insecure-cipher-algorithm-blowfish +# deepruleid:insecure-cipher-algorithm-blowfish msg = iv + cipher.encrypt(plaintext + padding) key = b'Sixteen byte key' diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-des.py b/python/pycryptodome/security/insecure-cipher-algorithm-des.py index 0455ab5592..bc25f6daac 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-des.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-des.py @@ -23,13 +23,13 @@ ctr = Counter.new(pycrypto_des.block_size*8/2, prefix=nonce) # ruleid:insecure-cipher-algorithm-des cipher = pycrypto_des.new(key, pycrypto_des.MODE_CTR, counter=ctr) -# deeptodoruleid:insecure-cipher-algorithm-des +# deepruleid:insecure-cipher-algorithm-des msg = nonce + cipher.encrypt(plaintext) nonce = Random.new().read(pycryptodomex_des.block_size/2) ctr = Counter.new(pycryptodomex_des.block_size*8/2, prefix=nonce) # ruleid:insecure-cipher-algorithm-des cipher = pycryptodomex_des.new(key, pycryptodomex_des.MODE_CTR, counter=ctr) -# deeptodoruleid:insecure-cipher-algorithm-des +# deepruleid:insecure-cipher-algorithm-des msg = nonce + cipher.encrypt(plaintext) diff --git a/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py b/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py index 145fc958b9..53d7d684c0 100644 --- a/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py +++ b/python/pycryptodome/security/insecure-cipher-algorithm-rc2.py @@ -19,11 +19,11 @@ iv = Random.new().read(pycrypto_arc2.block_size) # ruleid:insecure-cipher-algorithm-rc2 cipher = pycrypto_arc2.new(key, pycrypto_arc2.MODE_CFB, iv) -# deeptodoruleid:insecure-cipher-algorithm-rc2 +# deepruleid:insecure-cipher-algorithm-rc2 msg = iv + cipher.encrypt(b'Attack at dawn') # ruleid:insecure-cipher-algorithm-rc2 cipher = pycryptodomex_arc2.new(key, pycryptodomex_arc2.MODE_CFB, iv) -# deeptodoruleid:insecure-cipher-algorithm-rc2 +# deepruleid:insecure-cipher-algorithm-rc2 msg = iv + cipher.encrypt(b'Attack at dawn') key = b'Sixteen byte key' diff --git a/python/requests/security/no-auth-over-http.py b/python/requests/security/no-auth-over-http.py index 4df3b21edf..3977eb0283 100644 --- a/python/requests/security/no-auth-over-http.py +++ b/python/requests/security/no-auth-over-http.py @@ -2,7 +2,7 @@ # ok:no-auth-over-http good_url = "https://www.github.com" -# deeptodoruleid:no-auth-over-http +# deepruleid:no-auth-over-http bad_url = "http://www.github.com" # ruleid:no-auth-over-http diff --git a/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py b/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py index 71f1d38c57..7b25aacad2 100644 --- a/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py +++ b/python/sqlalchemy/security/sqlalchemy-execute-raw-query.py @@ -198,7 +198,7 @@ # ok: sqlalchemy-execute-raw-query engine = create_engine('postgresql://user@localhost/database') query = select(literal_column("users.fullname", String) + ', ' + literal_column("addresses.email_address").label("title")).where(and_(literal_column("users.id") == literal_column("addresses.user_id"), text("users.name BETWEEN 'm' AND 'z'"), text("(addresses.email_address LIKE :x OR addresses.email_address LIKE :y)"))).select_from(table('users')).select_from(table('addresses')) -# deeptodoruleid: sqlalchemy-execute-raw-query +# deepruleid: sqlalchemy-execute-raw-query conn.execute(query, {"x":"%@aol.com", "y":"%@msn.com"}).fetchall() diff --git a/ruby/rails/security/brakeman/check-sql.rb b/ruby/rails/security/brakeman/check-sql.rb index e6554f36cd..65e8e8c94f 100644 --- a/ruby/rails/security/brakeman/check-sql.rb +++ b/ruby/rails/security/brakeman/check-sql.rb @@ -147,7 +147,7 @@ def test_more_if_statements "blah" end - # ruleid: deeptodook: check-sql + # ruleid: deepok: check-sql Product.last("blah = '#{x}'") #ok: check-sql From 97bd5b07019065a5cd17c3cf1d7b6cc188cbe8bd Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Mon, 21 Oct 2024 09:47:34 +0200 Subject: [PATCH 22/28] Fix annots for osemgrep test --pro with DeepScan (#3492) test plan: osemgrep-pro test --pro semgrep-rules/ --- csharp/dotnet/security/use_ecb_mode.cs | 26 +++++++++---------- .../use_weak_rng_for_keygeneration.cs | 2 +- .../detect-disable-mustache-escape.js | 4 +-- .../delegatecall-to-arbitrary-address.sol | 2 +- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/csharp/dotnet/security/use_ecb_mode.cs b/csharp/dotnet/security/use_ecb_mode.cs index 452a6ec5b4..2eb4ca5ed3 100644 --- a/csharp/dotnet/security/use_ecb_mode.cs +++ b/csharp/dotnet/security/use_ecb_mode.cs @@ -5,7 +5,7 @@ public class Encryption { public void EncryptWithAesEcb() { Aes key = Aes.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var encryptor = key.CreateEncryptor(); byte[] msg = new byte[32]; @@ -15,13 +15,13 @@ public void EncryptWithAesEcb() { public void EncryptWithAesEcb2() { Aes key = Aes.Create(); byte[] msg = new byte[32]; - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var cipherText = key.EncryptEcb(msg, PaddingMode.PKCS7); } public void DecryptWithAesEcb(byte[] cipherText) { Aes key = Aes.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var decryptor = key.CreateDecryptor(); var msg = decryptor.TransformFinalBlock(cipherText, 0, cipherText.Length); @@ -29,13 +29,13 @@ public void DecryptWithAesEcb(byte[] cipherText) { public void DecryptWithAesEcb2(byte[] cipherText) { Aes key = Aes.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var msgText = key.DecryptEcb(cipherText, PaddingMode.PKCS7); } public void EncryptWith3DESEcb() { TripleDES key = TripleDES.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var encryptor = key.CreateEncryptor(); byte[] msg = new byte[32]; @@ -45,13 +45,13 @@ public void EncryptWith3DESEcb() { public void EncryptWith3DESEcb2() { TripleDES key = TripleDES.Create(); byte[] msg = new byte[32]; - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var cipherText = key.EncryptEcb(msg, PaddingMode.PKCS7); } public void DecryptWith3DESEcb(byte[] cipherText) { TripleDES key = TripleDES.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var decryptor = key.CreateDecryptor(); var msg = decryptor.TransformFinalBlock(cipherText, 0, cipherText.Length); @@ -59,12 +59,12 @@ public void DecryptWith3DESEcb(byte[] cipherText) { public void DecryptWith3DESEcb2(byte[] cipherText) { TripleDES key = TripleDES.Create(); - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var msgText = key.DecryptEcb(cipherText, PaddingMode.PKCS7); } public void EncryptWithEcb(SymmetricAlgorithm key) { - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var encryptor = key.CreateEncryptor(); byte[] msg = new byte[32]; @@ -73,19 +73,19 @@ public void EncryptWithEcb(SymmetricAlgorithm key) { public void EncryptWithEcb2(SymmetricAlgorithm key) { byte[] msg = new byte[32]; - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var cipherText = key.EncryptEcb(msg, PaddingMode.PKCS7); } public void DecryptWithEcb(SymmetricAlgorithm key, byte[] cipherText) { - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode key.Mode = CipherMode.ECB; using var decryptor = key.CreateDecryptor(); var msg = decryptor.TransformFinalBlock(cipherText, 0, cipherText.Length); } public void DecryptWithEcb2(SymmetricAlgorithm key, byte[] cipherText) { - //ruleid: use_ecb_mode + //ruleid: deeptodoruleid: use_ecb_mode var msgText = key.DecryptEcb(cipherText, PaddingMode.PKCS7); } @@ -124,4 +124,4 @@ public static void Main() { Console.WriteLine("Hello World"); } -} \ No newline at end of file +} diff --git a/csharp/dotnet/security/use_weak_rng_for_keygeneration.cs b/csharp/dotnet/security/use_weak_rng_for_keygeneration.cs index cd75ffe853..0da244ef47 100644 --- a/csharp/dotnet/security/use_weak_rng_for_keygeneration.cs +++ b/csharp/dotnet/security/use_weak_rng_for_keygeneration.cs @@ -8,7 +8,7 @@ public void GenerateBadKey() { byte[] key = new byte[16]; rng.NextBytes(key); SymmetricAlgorithm cipher = Aes.Create(); - // ruleid: use_weak_rng_for_keygeneration + // ruleid: deeptodoruleid: use_weak_rng_for_keygeneration cipher.Key = key; } diff --git a/javascript/lang/security/detect-disable-mustache-escape.js b/javascript/lang/security/detect-disable-mustache-escape.js index 052d45d309..583d8928d2 100644 --- a/javascript/lang/security/detect-disable-mustache-escape.js +++ b/javascript/lang/security/detect-disable-mustache-escape.js @@ -1,5 +1,5 @@ -// ruleid:detect-disable-mustache-escape +// ruleid: detect-disable-mustache-escape a.escapeMarkup = false; -// ok:detect-disable-mustache-escape +// deeptodook: detect-disable-mustache-escape escapeMarkup = false; diff --git a/solidity/security/delegatecall-to-arbitrary-address.sol b/solidity/security/delegatecall-to-arbitrary-address.sol index fe9d30b47a..71a8843486 100644 --- a/solidity/security/delegatecall-to-arbitrary-address.sol +++ b/solidity/security/delegatecall-to-arbitrary-address.sol @@ -36,7 +36,7 @@ contract Test{ function sink(address _contract, uint256 _num) internal { // this requires intraprocedural tainting (--pro-intrafile) - // proruleid: delegatecall-to-arbitrary-address + // proruleid: deeptodoruleid: delegatecall-to-arbitrary-address (bool success, bytes memory data) = _contract.delegatecall( abi.encodeWithSignature("setVars(uint256)", _num) ); From 0bba56c058e51adfb683954acafc8afb02e41467 Mon Sep 17 00:00:00 2001 From: Yoann Padioleau Date: Tue, 22 Oct 2024 10:46:14 +0200 Subject: [PATCH 23/28] Remove scripts/run-test to simplify, call just osemgrep test (#3493) * Remove scripts/run-test to simplify, call just osemgrep test It has been almost a month that we run both osemgrep test and pysemgrep --test and no complaints, so let's remove the use of pysemgrep --test so we can then remove the corresponding python code in pysemgrep. test plan: make validate make test-only wait for green CI checks * more --- .../workflows/semgrep-rules-test-develop.yml | 25 ++--- .github/workflows/semgrep-rules-test.yml | 8 +- Makefile | 58 ++++++++++- scripts/run-tests | 99 ------------------- 4 files changed, 66 insertions(+), 124 deletions(-) delete mode 100755 scripts/run-tests diff --git a/.github/workflows/semgrep-rules-test-develop.yml b/.github/workflows/semgrep-rules-test-develop.yml index 38e6a4ae03..2a759cc2d3 100644 --- a/.github/workflows/semgrep-rules-test-develop.yml +++ b/.github/workflows/semgrep-rules-test-develop.yml @@ -1,4 +1,4 @@ -# Running the tests in the repo using `semgrep test --experimental` and +# Running the tests in the repo using `semgrep test` (osemgrep) and # the semgrep/semgrep:pro-develop docker image (the bleeding edge!). name: semgrep-rules-test-develop @@ -14,27 +14,20 @@ on: jobs: test-develop: name: rules-test-develop + # alt: use directly the semgrep/semgrep:pro-develop container here so we + # don't need the calls to 'docker run ...' below runs-on: ubuntu-20.04 + # TODO: remove the with: path: below to simplify steps: - uses: actions/checkout@v2 with: path: semgrep-rules + # alt: call 'make validate' but would require 'make' in the docker image + # alt: export SEMGREP="docker run --rm -w ... semgrep" + # make -C "$GITHUB_WORKSPACE"/semgrep-rules validate + #TODO: this actually currently fails because of errors in stats/ but GHA + # still continue, weird - name: run osemgrep validate --pro run: docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep validate --pro . - name: run osemgrep test --pro run: docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep test --pro . - #TODO: we can delete all the rest below and also scripts/run-tests - - name: delete directories not containing rules - run: rm -rf semgrep-rules/stats - - name: delete rules requiring Semgrep Pro - run: rm -rf semgrep-rules/apex semgrep-rules/elixir - # TODO: this takes 1m20 in CI and could be optimized by switching to osemgrep - - name: validate rules - run: | - export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep" - make -C "$GITHUB_WORKSPACE"/semgrep-rules validate - # this now takes 21s with osemgrep instead of 3min with pysemgrep - - name: test with semgrep pro develop branch and with --experimental - run: | - export SEMGREP="docker run --rm -w /src -v ${GITHUB_WORKSPACE}/semgrep-rules:/src semgrep/semgrep:pro-develop semgrep --experimental" - make -C "$GITHUB_WORKSPACE"/semgrep-rules test-only diff --git a/.github/workflows/semgrep-rules-test.yml b/.github/workflows/semgrep-rules-test.yml index ac9672914e..5cd02cec82 100644 --- a/.github/workflows/semgrep-rules-test.yml +++ b/.github/workflows/semgrep-rules-test.yml @@ -17,13 +17,13 @@ jobs: - uses: actions/setup-python@v2 with: python-version: 3.9.2 - - name: install semgrep + - name: install semgrep via pip run: pip3 install semgrep - name: remove stats directory run: rm -rf stats - name: remove rules requiring Semgrep Pro run: rm -rf apex elixir - name: validate rules - run: semgrep --validate --config . - - name: run semgrep - run: semgrep --test --test-ignore-todo + run: semgrep validate . + - name: run semgrep test + run: semgrep test . diff --git a/Makefile b/Makefile index a6e9378e30..5d359b692a 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,65 @@ # # Check rule validity and check that semgrep finds the expected findings. +# See https://semgrep.dev/docs/writing-rules/testing-rules for more info. # -# The semgrep repo also runs this as part of its CI for consistency. +# The semgrep repo (and now semgrep-pro repo) also runs those tests as part +# of its CI for consistency. # .PHONY: test test: $(MAKE) validate $(MAKE) test-only -.PHONY: validate -validate: - ./scripts/run-tests validate +# Use the SEMGREP env variable to specify a non-standard semgrep command +SEMGREP ?= semgrep .PHONY: test-only +#old: pysemgrep --test was also using flags below but not needed +# --test-ignore-todo --strict --disable-version-check --metrics=off --verbose test-only: - ./scripts/run-tests test + $(SEMGREP) test --pro . + +# TODO: semgrep validate use a different targeting than 'semgrep test' +# so we unfortunately need this whitelist of dirs because it reports +# errors on stats/ and scripts/ (and .github/workflows/) files otherwise +# (we also skip libsonnet/ and trusted_python/ which do not contain rules) +LANG_DIRS=\ + bash \ + c \ + clojure \ + csharp \ + dockerfile \ + generic \ + go \ + html \ + java \ + javascript \ + json \ + kotlin \ + ocaml \ + php \ + python \ + ruby \ + rust \ + scala \ + solidity \ + swift \ + terraform \ + typescript \ + yaml +PRO_DIRS=apex elixir +OTHER_DIRS=ai problem-based-packs +DIRS=$(LANG_DIRS) $(PRO_DIRS) $(OTHER_DIRS) + +.PHONY: validate +#old: pysemgrep --validate was also using the flags below but not needed +# --strict --disable-version-check --metrics=off --verbose +validate: + $(SEMGREP) validate --pro $(DIRS) + +.PHONY: test-oss-only +test-oss-only: + @for dir in $(LANG_DIRS) $(OTHER_DIRS); do \ + echo "processing $$dir"; \ + $(SEMGREP) test $$dir; \ + done diff --git a/scripts/run-tests b/scripts/run-tests deleted file mode 100755 index c17e5f18d1..0000000000 --- a/scripts/run-tests +++ /dev/null @@ -1,99 +0,0 @@ -#! /usr/bin/env bash -# -# Validate rules and run semgrep to check that it's producing the expected -# results. -# -# This was originally part of the makefile but it's gotten too complicated -# due the need for extra escaping and error codes being ignored. -# -set -eu - -usage() { - cat <&2 - exit 1 -} - -# Use the SEMGREP environment variable to specify a non-standard semgrep -# command. This is useful for calling a development version of semgrep -# e.g. -# PIPENV_PIPFILE=~/semgrep/cli/Pipfile SEMGREP='pipenv run semgrep' make test -# -if [[ -z "${SEMGREP-}" ]]; then - SEMGREP="semgrep" -fi - -# Obtain the list of folders containing Semgrep rule files -# (with a .yml suffix). -# -# This is meant to exclude folders that don't contain rule files and -# may contain .yml files that are not Semgrep rules and would result -# in errors. -# -# Skipping the "Apex" and "Elixir" folders because it will require splitting test logic -# to run Semgrep OSS and Semgrep Pro with different expected results. -# -set_rule_folders() { - rule_folders=$(find . -mindepth 1 -maxdepth 1 -type d \ - | grep -v '^./\(\..*\|stats\|trusted_python\|scripts\|libsonnet\|apex\|elixir\)/\?$' \ - | sort) - if [[ -z "$rule_folders" ]]; then - error "Cannot find any rule folders to scan in $(pwd)" - fi -} - -# stdout is redirected to stderr so as to see the whole output in the correct -# order when running this from pytest. -# (pytest captures stdout and stderr separately and prints them later, -# if at all). -# -validate() { - set_rule_folders - for root in $rule_folders; do - echo "======== validate rule files in $root ========" - time -p $SEMGREP --validate \ - --strict --disable-version-check \ - --metrics=off --verbose \ - --config="./$root" - done >&2 -} - -test_only() { - set_rule_folders - for root in $rule_folders; do - echo "========= test rules in $root =========" - time -p $SEMGREP --test --test-ignore-todo \ - --strict --disable-version-check \ - --metrics=off --verbose \ - "./$root" - done >&2 -} - -if [[ $# != 1 ]]; then - usage >&2 - exit 1 -else - case "$1" in - validate) - validate - exit 0 - ;; - test) - test_only - exit 0 - ;; - *) - usage >&2 - exit 1 - esac -fi From aa55fb53e6e6f68f16cf5eaa99d05869a4a578a5 Mon Sep 17 00:00:00 2001 From: Vasilii Ermilov Date: Thu, 24 Oct 2024 18:47:58 +0800 Subject: [PATCH 24/28] remove redundant rules for HTML templates (#3349) * remove redundant rules for HTML templates * Delete python/django/security/audit/xss/var-in-script-tag.html * Delete python/django/security/audit/xss/var-in-script-tag.yaml --------- Co-authored-by: Claudio --- .../audit/xss/mustache/var-in-href.mustache | 62 ------------------- .../audit/xss/mustache/var-in-href.yaml | 38 ------------ .../security/audit/xss/template-href-var.html | 22 ------- .../security/audit/xss/template-href-var.yaml | 41 ------------ .../security/audit/xss/var-in-script-tag.html | 21 ------- .../security/audit/xss/var-in-script-tag.yaml | 43 ------------- .../security/xss/audit/template-href-var.html | 32 ---------- .../security/xss/audit/template-href-var.yaml | 43 ------------- 8 files changed, 302 deletions(-) delete mode 100644 javascript/express/security/audit/xss/mustache/var-in-href.mustache delete mode 100644 javascript/express/security/audit/xss/mustache/var-in-href.yaml delete mode 100644 python/django/security/audit/xss/template-href-var.html delete mode 100644 python/django/security/audit/xss/template-href-var.yaml delete mode 100644 python/django/security/audit/xss/var-in-script-tag.html delete mode 100644 python/django/security/audit/xss/var-in-script-tag.yaml delete mode 100644 python/flask/security/xss/audit/template-href-var.html delete mode 100644 python/flask/security/xss/audit/template-href-var.yaml diff --git a/javascript/express/security/audit/xss/mustache/var-in-href.mustache b/javascript/express/security/audit/xss/mustache/var-in-href.mustache deleted file mode 100644 index b47e8200f0..0000000000 --- a/javascript/express/security/audit/xss/mustache/var-in-href.mustache +++ /dev/null @@ -1,62 +0,0 @@ - - - - - - Demo Mustache.JS - - - - - - - - - -
-
-
-
- - -

Singed in as: {{ val }}

- - - - - - - diff --git a/javascript/express/security/audit/xss/mustache/var-in-href.yaml b/javascript/express/security/audit/xss/mustache/var-in-href.yaml deleted file mode 100644 index 6f6148dca9..0000000000 --- a/javascript/express/security/audit/xss/mustache/var-in-href.yaml +++ /dev/null @@ -1,38 +0,0 @@ -rules: -- id: var-in-href - message: >- - Detected a template variable used in an anchor tag with - the 'href' attribute. This allows a malicious actor to - input the 'javascript:' URI and is subject to cross- - site scripting (XSS) attacks. If using a relative URL, - start with a literal forward slash and concatenate the URL, - like this: href='/{{link}}'. You may also consider setting - the Content Security Policy (CSP) header. - metadata: - cwe: - - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: - - A07:2017 - Cross-Site Scripting (XSS) - - A03:2021 - Injection - references: - - https://flask.palletsprojects.com/en/1.1.x/security/#cross-site-scripting-xss#:~:text=javascript:%20URI - - https://github.com/pugjs/pug/issues/2952 - category: security - technology: - - express - cwe2022-top25: true - cwe2021-top25: true - subcategory: - - audit - likelihood: LOW - impact: MEDIUM - confidence: LOW - languages: - - regex - severity: WARNING - paths: - include: - - '*.mustache' - - '*.hbs' - - '*.html' - pattern-regex: From: {{ from_email }} -

To: - {% for recipient in recipients %} - {{ recipient }}  - {% endfor %} -

-

Subject: {{subject}}

- - -
diff --git a/python/django/security/audit/xss/template-href-var.yaml b/python/django/security/audit/xss/template-href-var.yaml deleted file mode 100644 index 5069f3952d..0000000000 --- a/python/django/security/audit/xss/template-href-var.yaml +++ /dev/null @@ -1,41 +0,0 @@ -rules: -- id: template-href-var - message: >- - Detected a template variable used in an anchor tag with - the 'href' attribute. This allows a malicious actor to - input the 'javascript:' URI and is subject to cross- - site scripting (XSS) attacks. Use the 'url' template tag - to safely generate a URL. You may also consider setting - the Content Security Policy (CSP) header. - metadata: - cwe: - - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: - - A07:2017 - Cross-Site Scripting (XSS) - - A03:2021 - Injection - references: - - https://flask.palletsprojects.com/en/1.1.x/security/#cross-site-scripting-xss - - https://docs.djangoproject.com/en/3.1/ref/templates/builtins/#url - - https://content-security-policy.com/ - category: security - technology: - - django - cwe2022-top25: true - cwe2021-top25: true - subcategory: - - audit - likelihood: LOW - impact: MEDIUM - confidence: LOW - languages: - - generic - paths: - include: - - '*.html' - severity: WARNING - patterns: - - pattern-inside: - - pattern-either: - - pattern: href = '{{...}}' - - pattern: href = "{{...}}" - - pattern: href = {{...}} diff --git a/python/django/security/audit/xss/var-in-script-tag.html b/python/django/security/audit/xss/var-in-script-tag.html deleted file mode 100644 index 8bfb72f8d5..0000000000 --- a/python/django/security/audit/xss/var-in-script-tag.html +++ /dev/null @@ -1,21 +0,0 @@ - - - - - -
- -

{{ this_is_fine }}

-
- - - - - diff --git a/python/django/security/audit/xss/var-in-script-tag.yaml b/python/django/security/audit/xss/var-in-script-tag.yaml deleted file mode 100644 index a56724ba50..0000000000 --- a/python/django/security/audit/xss/var-in-script-tag.yaml +++ /dev/null @@ -1,43 +0,0 @@ -rules: -- id: var-in-script-tag - languages: [generic] - severity: ERROR - message: >- - Detected a template variable used in a script tag. - Although template variables are HTML escaped, HTML - escaping does not always prevent cross-site scripting (XSS) - attacks when used directly in JavaScript. If you need this - data on the rendered page, consider placing it in the HTML - portion (outside of a script tag). Alternatively, use a - JavaScript-specific encoder, such as the one available - in OWASP ESAPI. For Django, you may also consider using - the 'json_script' template tag and retrieving the data in - your script by using the element ID (e.g., `document.getElementById`). - patterns: - - pattern-inside: - - pattern: '{{ ... }}' - - pattern-not-inside: nonce = '...' - - pattern-not-inside: nonce = "..." - paths: - include: - - '*.html' - metadata: - cwe: - - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: - - A07:2017 - Cross-Site Scripting (XSS) - - A03:2021 - Injection - references: - - https://adamj.eu/tech/2020/02/18/safely-including-data-for-javascript-in-a-django-template/?utm_campaign=Django%2BNewsletter&utm_medium=rss&utm_source=Django_Newsletter_12A - - https://www.veracode.com/blog/secure-development/nodejs-template-engines-why-default-encoders-are-not-enough - - https://github.com/ESAPI/owasp-esapi-js - category: security - technology: - - django - cwe2022-top25: true - cwe2021-top25: true - subcategory: - - audit - likelihood: LOW - impact: MEDIUM - confidence: LOW diff --git a/python/flask/security/xss/audit/template-href-var.html b/python/flask/security/xss/audit/template-href-var.html deleted file mode 100644 index 34adbcf891..0000000000 --- a/python/flask/security/xss/audit/template-href-var.html +++ /dev/null @@ -1,32 +0,0 @@ -

From: {{ from_email }}

-

To: - {% for recipient in recipients %} - {{ recipient }}  - {% endfor %} -

-

Subject: {{subject}}

- - -
diff --git a/python/flask/security/xss/audit/template-href-var.yaml b/python/flask/security/xss/audit/template-href-var.yaml deleted file mode 100644 index d8d625198f..0000000000 --- a/python/flask/security/xss/audit/template-href-var.yaml +++ /dev/null @@ -1,43 +0,0 @@ -rules: -- id: template-href-var - message: >- - Detected a template variable used in an anchor tag with - the 'href' attribute. This allows a malicious actor to - input the 'javascript:' URI and is subject to cross- - site scripting (XSS) attacks. Use 'url_for()' to safely - generate a URL. You may also consider setting the Content - Security Policy (CSP) header. - metadata: - cwe: - - "CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')" - owasp: - - A07:2017 - Cross-Site Scripting (XSS) - - A03:2021 - Injection - references: - - https://flask.palletsprojects.com/en/1.1.x/security/#cross-site-scripting-xss - - https://content-security-policy.com/ - category: security - technology: - - flask - cwe2022-top25: true - cwe2021-top25: true - subcategory: - - audit - likelihood: LOW - impact: MEDIUM - confidence: LOW - languages: - - generic - paths: - include: - - '*.html' - severity: WARNING - patterns: - - pattern-inside: - - pattern-either: - - pattern: href = {{ ... }} - - pattern: href = "{{ ... }}" - - pattern: href = '{{ ... }}' - - pattern-not-inside: href = {{ url_for(...) ... }} - - pattern-not-inside: href = "{{ url_for(...) ... }}" - - pattern-not-inside: href = '{{ url_for(...) ... }}' From e91dd3fd59680bca7f5e9a381882994213a17ac4 Mon Sep 17 00:00:00 2001 From: Claudio Date: Thu, 24 Oct 2024 16:34:52 +0200 Subject: [PATCH 25/28] Update stacktrace-disclosure rule and test (#3495) * Update stacktrace-disclosure.cs * Update stacktrace-disclosure.yaml --- csharp/lang/security/stacktrace-disclosure.cs | 7 +++++++ csharp/lang/security/stacktrace-disclosure.yaml | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/csharp/lang/security/stacktrace-disclosure.cs b/csharp/lang/security/stacktrace-disclosure.cs index 9c3bab216c..ffeb42f457 100644 --- a/csharp/lang/security/stacktrace-disclosure.cs +++ b/csharp/lang/security/stacktrace-disclosure.cs @@ -24,3 +24,10 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) app.UseExceptionHandler("/Error"); } } + +public void Configure(IApplicationBuilder app, IWebHostEnvironment env) +{ + if (env.IsDevelopment()) + // ok: stacktrace-disclosure + app.UseDeveloperExceptionPage(); +} diff --git a/csharp/lang/security/stacktrace-disclosure.yaml b/csharp/lang/security/stacktrace-disclosure.yaml index 1337b8582b..882989f288 100644 --- a/csharp/lang/security/stacktrace-disclosure.yaml +++ b/csharp/lang/security/stacktrace-disclosure.yaml @@ -4,8 +4,6 @@ rules: - pattern: $APP.UseDeveloperExceptionPage(...); - pattern-not-inside: | if ($ENV.IsDevelopment(...)) { - ... - $APP.UseDeveloperExceptionPage(...); ... } message: >- From 157ae4706d7f54aa33647332f234dd68f30da195 Mon Sep 17 00:00:00 2001 From: Claudio Date: Thu, 24 Oct 2024 18:48:38 +0200 Subject: [PATCH 26/28] Remove redundant rule python.lang.security.audit.ftplib (#3496) * Remove redundant rule python.lang.security.audit.ftplib python.lang.security.audit.ftplib.ftplib is best replaced by python.lang.security.audit.insecure-transport.ftplib.use-ftp-tls.use-ftp-tls * Update use-ftp-tls.yaml --- python/lang/security/audit/ftplib.yaml | 27 ------------------- .../ftplib/use-ftp-tls.yaml | 2 +- 2 files changed, 1 insertion(+), 28 deletions(-) delete mode 100644 python/lang/security/audit/ftplib.yaml diff --git a/python/lang/security/audit/ftplib.yaml b/python/lang/security/audit/ftplib.yaml deleted file mode 100644 index a002121899..0000000000 --- a/python/lang/security/audit/ftplib.yaml +++ /dev/null @@ -1,27 +0,0 @@ -rules: -- id: ftplib - pattern: ftplib.$ANYTHING(...) - message: >- - FTP does not encrypt communications by default. This can lead to sensitive - data being exposed. Ensure use of FTP here does not expose sensitive data. - metadata: - source-rule-url: https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/bandit/blacklists/calls.py#L265 - cwe: - - 'CWE-319: Cleartext Transmission of Sensitive Information' - owasp: - - A03:2017 - Sensitive Data Exposure - - A02:2021 - Cryptographic Failures - bandit-code: B321 - references: - - https://docs.python.org/3/library/telnetlib.html - category: security - technology: - - ftplib - subcategory: - - audit - likelihood: LOW - impact: MEDIUM - confidence: LOW - severity: WARNING - languages: - - python diff --git a/python/lang/security/audit/insecure-transport/ftplib/use-ftp-tls.yaml b/python/lang/security/audit/insecure-transport/ftplib/use-ftp-tls.yaml index 11beea5840..ee2fbaa4b6 100644 --- a/python/lang/security/audit/insecure-transport/ftplib/use-ftp-tls.yaml +++ b/python/lang/security/audit/insecure-transport/ftplib/use-ftp-tls.yaml @@ -35,5 +35,5 @@ rules: likelihood: LOW impact: LOW confidence: LOW - severity: WARNING + severity: INFO languages: [python] From 116b0bd0314b85aa617e1392cbb813f23c73aaee Mon Sep 17 00:00:00 2001 From: Austin Theriault Date: Tue, 29 Oct 2024 09:55:29 -0700 Subject: [PATCH 27/28] Delete python/lang/security/audit/ftplib.py Should have been deleted in https://github.com/semgrep/semgrep-rules/pull/3496, causing https://github.com/semgrep/semgrep-proprietary/pull/2505 to fail --- python/lang/security/audit/ftplib.py | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 python/lang/security/audit/ftplib.py diff --git a/python/lang/security/audit/ftplib.py b/python/lang/security/audit/ftplib.py deleted file mode 100644 index cd131ca7cd..0000000000 --- a/python/lang/security/audit/ftplib.py +++ /dev/null @@ -1,12 +0,0 @@ -# cf. https://github.com/PyCQA/bandit/blob/d5f8fa0d89d7b11442fc6ec80ca42953974354c8/examples/ftplib.py - -from ftplib import FTP - -# ruleid:ftplib -ftp = FTP('ftp.debian.org') -ftp.login() - -ftp.cwd('debian') -ftp.retrlines('LIST') - -ftp.quit() From 178f46ebf4902fdbf748d0dcddb9d15401352312 Mon Sep 17 00:00:00 2001 From: Claudio Date: Mon, 4 Nov 2024 18:35:57 +0100 Subject: [PATCH 28/28] Fix openai-consequential-action-false metadata (#3509) --- yaml/openapi/security/openai-consequential-action-false.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/yaml/openapi/security/openai-consequential-action-false.yaml b/yaml/openapi/security/openai-consequential-action-false.yaml index d54163e597..45dc49c031 100644 --- a/yaml/openapi/security/openai-consequential-action-false.yaml +++ b/yaml/openapi/security/openai-consequential-action-false.yaml @@ -38,6 +38,7 @@ rules: likelihood: HIGH impact: HIGH confidence: HIGH + cwe: "CWE-441: Unintended Proxy or Intermediary ('Confused Deputy')" owasp: - 'A04:2021 Insecure Design' - 'LLM08:2023 - Excessive Agency'