From b91f8ac6a193f088c10e3d2a22a023238c3008a0 Mon Sep 17 00:00:00 2001 From: David Roe Date: Mon, 9 Oct 2023 14:44:28 +0100 Subject: [PATCH] feat: add symfony open redirect rule --- rules/php/shared/lang/user_input.yml | 8 +- rules/php/shared/symfony/user_input.yml | 14 ++++ rules/php/symfony/open_redirect.yml | 42 ++++++++++ .../open_redirect/__snapshots__/test.js.snap | 78 +++++++++++++++++++ tests/php/symfony/open_redirect/test.js | 16 ++++ .../symfony/open_redirect/testdata/bad.php | 18 +++++ .../php/symfony/open_redirect/testdata/ok.php | 18 +++++ 7 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 rules/php/shared/symfony/user_input.yml create mode 100644 rules/php/symfony/open_redirect.yml create mode 100644 tests/php/symfony/open_redirect/__snapshots__/test.js.snap create mode 100644 tests/php/symfony/open_redirect/test.js create mode 100644 tests/php/symfony/open_redirect/testdata/bad.php create mode 100644 tests/php/symfony/open_redirect/testdata/ok.php diff --git a/rules/php/shared/lang/user_input.yml b/rules/php/shared/lang/user_input.yml index 84bab6820..142a0d9f6 100644 --- a/rules/php/shared/lang/user_input.yml +++ b/rules/php/shared/lang/user_input.yml @@ -1,6 +1,8 @@ type: shared languages: - php +imports: + - php_shared_symfony_user_input sanitizer: php_shared_lang_user_input_sanitized patterns: - pattern: $$[$<_>] @@ -9,7 +11,11 @@ patterns: values: - _GET - _POST - + - pattern: $; + filters: + - variable: SYMFONY_USER_INPUT + detection: php_shared_symfony_user_input + scope: cursor_strict auxiliary: - id: php_shared_lang_user_input_sanitized patterns: diff --git a/rules/php/shared/symfony/user_input.yml b/rules/php/shared/symfony/user_input.yml new file mode 100644 index 000000000..13172163c --- /dev/null +++ b/rules/php/shared/symfony/user_input.yml @@ -0,0 +1,14 @@ +type: shared +languages: + - php +patterns: + - pattern: | + class $ { + public function $<_>($<...>$$<_>$<...>) {} + } + filters: + - variable: CLASS_NAME + regex: Controller\z +metadata: + description: "PHP user input." + id: php_shared_symfony_user_input diff --git a/rules/php/symfony/open_redirect.yml b/rules/php/symfony/open_redirect.yml new file mode 100644 index 000000000..69bfe35e4 --- /dev/null +++ b/rules/php/symfony/open_redirect.yml @@ -0,0 +1,42 @@ +imports: + - php_shared_lang_user_input +patterns: + - pattern: $this->$($$<...>) + filters: + - variable: METHOD + values: + - redirect + - redirectToRoute + - variable: USER_INPUT + detection: php_shared_lang_user_input + scope: result +languages: + - php +severity: medium +metadata: + description: "Open redirect detected." + remediation_message: | + ## Description + A redirect using unsanitized user input is bad practice and puts your application at greater risk of phishing attacks. + + ## Remediations + ❌ Do not use unsanitized user input when constructing URLs + + ✅ Instead, ensure the input is validated by using a safe list or a mapping when constructing URLs + + ```php + $paths = [ + "1" => "/planes", + "2" => "/trains", + "3" => "/automobiles", + ]; + + $transport = $_GET["transport"]; + $this->redirect($paths[$transport]"); + ``` + ## Resources + - [OWASP open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) + cwe_id: + - 601 + id: "php_symfony_open_redirect" + documentation_url: https://docs.bearer.com/reference/rules/php_symfony_open_redirect diff --git a/tests/php/symfony/open_redirect/__snapshots__/test.js.snap b/tests/php/symfony/open_redirect/__snapshots__/test.js.snap new file mode 100644 index 000000000..fb3a3557c --- /dev/null +++ b/tests/php/symfony/open_redirect/__snapshots__/test.js.snap @@ -0,0 +1,78 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`php_symfony_open_redirect bad 1`] = ` +"{ + "medium": [ + { + "cwe_ids": [ + "601" + ], + "id": "php_symfony_open_redirect", + "title": "Open redirect detected.", + "description": "## Description\\nA redirect using unsanitized user input is bad practice and puts your application at greater risk of phishing attacks.\\n\\n## Remediations\\n❌ Do not use unsanitized user input when constructing URLs\\n\\n✅ Instead, ensure the input is validated by using a safe list or a mapping when constructing URLs\\n\\n\`\`\`php\\n $paths = [\\n \\"1\\" => \\"/planes\\",\\n \\"2\\" => \\"/trains\\",\\n \\"3\\" => \\"/automobiles\\",\\n ];\\n\\n $transport = $_GET[\\"transport\\"];\\n $this->redirect($paths[$transport]\\");\\n\`\`\`\\n## Resources\\n- [OWASP open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_open_redirect", + "line_number": 11, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 11, + "end": 11, + "column": { + "start": 12, + "end": 31 + } + }, + "sink": { + "start": 11, + "end": 11, + "column": { + "start": 12, + "end": 31 + }, + "content": "$this->redirect($x)" + }, + "parent_line_number": 11, + "snippet": "$this->redirect($x)", + "fingerprint": "d582d9374c5c887f53bc5285ec981b5d_0", + "old_fingerprint": "edb8291f0e96f91996f8aaf9a772d55b_0", + "code_extract": " return $this->redirect($x);" + }, + { + "cwe_ids": [ + "601" + ], + "id": "php_symfony_open_redirect", + "title": "Open redirect detected.", + "description": "## Description\\nA redirect using unsanitized user input is bad practice and puts your application at greater risk of phishing attacks.\\n\\n## Remediations\\n❌ Do not use unsanitized user input when constructing URLs\\n\\n✅ Instead, ensure the input is validated by using a safe list or a mapping when constructing URLs\\n\\n\`\`\`php\\n $paths = [\\n \\"1\\" => \\"/planes\\",\\n \\"2\\" => \\"/trains\\",\\n \\"3\\" => \\"/automobiles\\",\\n ];\\n\\n $transport = $_GET[\\"transport\\"];\\n $this->redirect($paths[$transport]\\");\\n\`\`\`\\n## Resources\\n- [OWASP open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_open_redirect", + "line_number": 16, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 16, + "end": 16, + "column": { + "start": 12, + "end": 53 + } + }, + "sink": { + "start": 16, + "end": 16, + "column": { + "start": 12, + "end": 53 + }, + "content": "$this->redirectToRoute($_GET[\\"oops\\"], [])" + }, + "parent_line_number": 16, + "snippet": "$this->redirectToRoute($_GET[\\"oops\\"], [])", + "fingerprint": "d582d9374c5c887f53bc5285ec981b5d_1", + "old_fingerprint": "edb8291f0e96f91996f8aaf9a772d55b_1", + "code_extract": " return $this->redirectToRoute($_GET[\\"oops\\"], []);" + } + ] +}" +`; + +exports[`php_symfony_open_redirect ok 1`] = `"{}"`; diff --git a/tests/php/symfony/open_redirect/test.js b/tests/php/symfony/open_redirect/test.js new file mode 100644 index 000000000..bca0fa61e --- /dev/null +++ b/tests/php/symfony/open_redirect/test.js @@ -0,0 +1,16 @@ +const { createInvoker, getEnvironment } = require("../../../helper.js") +const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) + +describe(ruleId, () => { + const invoke = createInvoker(ruleId, ruleFile, testBase) + + test("bad", () => { + const testCase = "bad.php" + expect(invoke(testCase)).toMatchSnapshot() + }) + + test("ok", () => { + const testCase = "ok.php" + expect(invoke(testCase)).toMatchSnapshot() + }) +}) diff --git a/tests/php/symfony/open_redirect/testdata/bad.php b/tests/php/symfony/open_redirect/testdata/bad.php new file mode 100644 index 000000000..684735d93 --- /dev/null +++ b/tests/php/symfony/open_redirect/testdata/bad.php @@ -0,0 +1,18 @@ +redirect($x); + } + + #[Route('/bar')] + public function bar(): Response { + return $this->redirectToRoute($_GET["oops"], []); + } +} diff --git a/tests/php/symfony/open_redirect/testdata/ok.php b/tests/php/symfony/open_redirect/testdata/ok.php new file mode 100644 index 000000000..80e5129d6 --- /dev/null +++ b/tests/php/symfony/open_redirect/testdata/ok.php @@ -0,0 +1,18 @@ +redirect($ok); + } + + #[Route('/bar')] + public function bar(): Response { + return $this->redirectToRoute($ok, []); + } +}