From 9ccac6c928c2a287e2568e14e4e7fae737ba1ab6 Mon Sep 17 00:00:00 2001 From: David Roe Date: Tue, 10 Oct 2023 14:21:41 +0100 Subject: [PATCH] feat: session key using user input rules --- .../php/lang/session_key_using_user_input.yml | 27 +++ rules/php/shared/lang/instance.yml | 10 +- .../symfony/session_key_using_user_input.yml | 56 +++++ .../__snapshots__/test.js.snap | 44 ++++ .../lang/session_key_using_user_input/test.js | 16 ++ .../testdata/bad.php | 5 + .../testdata/ok.php | 7 + .../__snapshots__/test.js.snap | 214 ++++++++++++++++++ .../session_key_using_user_input/test.js | 16 ++ .../testdata/bad.php | 36 +++ .../testdata/ok.php | 36 +++ 11 files changed, 466 insertions(+), 1 deletion(-) create mode 100644 rules/php/lang/session_key_using_user_input.yml create mode 100644 rules/php/symfony/session_key_using_user_input.yml create mode 100644 tests/php/lang/session_key_using_user_input/__snapshots__/test.js.snap create mode 100644 tests/php/lang/session_key_using_user_input/test.js create mode 100644 tests/php/lang/session_key_using_user_input/testdata/bad.php create mode 100644 tests/php/lang/session_key_using_user_input/testdata/ok.php create mode 100644 tests/php/symfony/session_key_using_user_input/__snapshots__/test.js.snap create mode 100644 tests/php/symfony/session_key_using_user_input/test.js create mode 100644 tests/php/symfony/session_key_using_user_input/testdata/bad.php create mode 100644 tests/php/symfony/session_key_using_user_input/testdata/ok.php diff --git a/rules/php/lang/session_key_using_user_input.yml b/rules/php/lang/session_key_using_user_input.yml new file mode 100644 index 000000000..56d5151ad --- /dev/null +++ b/rules/php/lang/session_key_using_user_input.yml @@ -0,0 +1,27 @@ +imports: + - php_shared_lang_user_input +patterns: + - pattern: $_SESSION[$] + filters: + - variable: USER_INPUT + detection: php_shared_lang_user_input + scope: result +languages: + - php +severity: high +metadata: + description: "User input detected in a session key." + remediation_message: | + ## Description + Using user-defined data in a session key is bad practice and can allow + attackers to perform unsafe actions. + + ## Remediations + ❌ Avoid using user-defined data in session keys + + ## Resources + - [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html) + cwe_id: + - 276 + id: php_lang_session_key_using_user_input + documentation_url: https://docs.bearer.com/reference/rules/php_lang_session_key_using_user_input diff --git a/rules/php/shared/lang/instance.yml b/rules/php/shared/lang/instance.yml index 8c4b6266c..d8413f657 100644 --- a/rules/php/shared/lang/instance.yml +++ b/rules/php/shared/lang/instance.yml @@ -2,7 +2,15 @@ type: shared languages: - php patterns: - - pattern: new $; + - new $; + - | + class $<_> { + public function $<_>($<...>$ $$<_>$<...>) {} + } + - | + class $<_> { + public function __construct($<...>public $ $$<_>$<...>) {} + } metadata: description: "PHP instance." id: php_shared_lang_instance diff --git a/rules/php/symfony/session_key_using_user_input.yml b/rules/php/symfony/session_key_using_user_input.yml new file mode 100644 index 000000000..a0fcd74ab --- /dev/null +++ b/rules/php/symfony/session_key_using_user_input.yml @@ -0,0 +1,56 @@ +imports: + - php_shared_lang_user_input + - php_shared_lang_instance +patterns: + - pattern: $->$($$<...>) + filters: + - variable: SESSION + detection: php_symfony_session_key_using_user_input_session + scope: cursor + - variable: METHOD + values: + - get + - put + - variable: USER_INPUT + detection: php_shared_lang_user_input + scope: result +auxiliary: + - id: php_symfony_session_key_using_user_input_session + patterns: + - pattern: $->getSession() + filters: + - variable: REQUEST_STACK + detection: php_shared_lang_instance + scope: cursor + filters: + - variable: CLASS + regex: \A(Symfony\\Component\\HttpFoundation\\)?RequestStack\z + - pattern: $->getSession() + filters: + - variable: REQUEST + detection: php_shared_lang_instance + scope: cursor + filters: + - variable: CLASS + regex: \A(Symfony\\Component\\HttpFoundation\\)?Request\z + # fallback until we have instance variable support + - $<_>->getSession() +languages: + - php +severity: high +metadata: + description: "User input detected in a session key." + remediation_message: | + ## Description + Using user-defined data in a session key is bad practice and can allow + attackers to perform unsafe actions. + + ## Remediations + ❌ Avoid using user-defined data in session keys + + ## Resources + - [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html) + cwe_id: + - 276 + id: php_symfony_session_key_using_user_input + documentation_url: https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input diff --git a/tests/php/lang/session_key_using_user_input/__snapshots__/test.js.snap b/tests/php/lang/session_key_using_user_input/__snapshots__/test.js.snap new file mode 100644 index 000000000..1f7fe0293 --- /dev/null +++ b/tests/php/lang/session_key_using_user_input/__snapshots__/test.js.snap @@ -0,0 +1,44 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`php_lang_session_key_using_user_input bad 1`] = ` +"{ + "high": [ + { + "cwe_ids": [ + "276" + ], + "id": "php_lang_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_lang_session_key_using_user_input", + "line_number": 5, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 5, + "end": 5, + "column": { + "start": 6, + "end": 27 + } + }, + "sink": { + "start": 5, + "end": 5, + "column": { + "start": 6, + "end": 27 + }, + "content": "$_SESSION[$userInput]" + }, + "parent_line_number": 5, + "snippet": "$_SESSION[$userInput]", + "fingerprint": "45a4ba0fe307692b0fc18a487074b1d5_0", + "old_fingerprint": "5b3a52e61b58df6745c36dd832d1e655_0", + "code_extract": "call($_SESSION[$userInput]);" + } + ] +}" +`; + +exports[`php_lang_session_key_using_user_input ok 1`] = `"{}"`; diff --git a/tests/php/lang/session_key_using_user_input/test.js b/tests/php/lang/session_key_using_user_input/test.js new file mode 100644 index 000000000..de4ba2b05 --- /dev/null +++ b/tests/php/lang/session_key_using_user_input/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("ok", () => { + const testCase = "ok.php" + expect(invoke(testCase)).toMatchSnapshot() + }) + + test("bad", () => { + const testCase = "bad.php" + expect(invoke(testCase)).toMatchSnapshot() + }) +}) diff --git a/tests/php/lang/session_key_using_user_input/testdata/bad.php b/tests/php/lang/session_key_using_user_input/testdata/bad.php new file mode 100644 index 000000000..262a0aad5 --- /dev/null +++ b/tests/php/lang/session_key_using_user_input/testdata/bad.php @@ -0,0 +1,5 @@ +get($userInput, [])" + }, + "parent_line_number": 13, + "snippet": "$session->get($userInput, [])", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_0", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_0", + "code_extract": " $session->get($userInput, []);" + }, + { + "cwe_ids": [ + "276" + ], + "id": "php_symfony_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input", + "line_number": 14, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 14, + "end": 14, + "column": { + "start": 9, + "end": 41 + } + }, + "sink": { + "start": 14, + "end": 14, + "column": { + "start": 9, + "end": 41 + }, + "content": "$session->put($userInput, $data)" + }, + "parent_line_number": 14, + "snippet": "$session->put($userInput, $data)", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_1", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_1", + "code_extract": " $session->put($userInput, $data);" + }, + { + "cwe_ids": [ + "276" + ], + "id": "php_symfony_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input", + "line_number": 20, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 20, + "end": 20, + "column": { + "start": 9, + "end": 38 + } + }, + "sink": { + "start": 20, + "end": 20, + "column": { + "start": 9, + "end": 38 + }, + "content": "$session->get($userInput, [])" + }, + "parent_line_number": 20, + "snippet": "$session->get($userInput, [])", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_2", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_2", + "code_extract": " $session->get($userInput, []);" + }, + { + "cwe_ids": [ + "276" + ], + "id": "php_symfony_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input", + "line_number": 21, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 21, + "end": 21, + "column": { + "start": 9, + "end": 41 + } + }, + "sink": { + "start": 21, + "end": 21, + "column": { + "start": 9, + "end": 41 + }, + "content": "$session->put($userInput, $data)" + }, + "parent_line_number": 21, + "snippet": "$session->put($userInput, $data)", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_3", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_3", + "code_extract": " $session->put($userInput, $data);" + }, + { + "cwe_ids": [ + "276" + ], + "id": "php_symfony_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input", + "line_number": 33, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 33, + "end": 33, + "column": { + "start": 5, + "end": 34 + } + }, + "sink": { + "start": 33, + "end": 33, + "column": { + "start": 5, + "end": 34 + }, + "content": "$session->get($userInput, [])" + }, + "parent_line_number": 33, + "snippet": "$session->get($userInput, [])", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_4", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_4", + "code_extract": " $session->get($userInput, []);" + }, + { + "cwe_ids": [ + "276" + ], + "id": "php_symfony_session_key_using_user_input", + "title": "User input detected in a session key.", + "description": "## Description\\nUsing user-defined data in a session key is bad practice and can allow\\nattackers to perform unsafe actions.\\n\\n## Remediations\\n❌ Avoid using user-defined data in session keys\\n\\n## Resources\\n- [OWASP Session management cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html)\\n", + "documentation_url": "https://docs.bearer.com/reference/rules/php_symfony_session_key_using_user_input", + "line_number": 34, + "full_filename": "/tmp/bearer-scan/bad.php", + "filename": ".", + "source": { + "start": 34, + "end": 34, + "column": { + "start": 5, + "end": 37 + } + }, + "sink": { + "start": 34, + "end": 34, + "column": { + "start": 5, + "end": 37 + }, + "content": "$session->put($userInput, $data)" + }, + "parent_line_number": 34, + "snippet": "$session->put($userInput, $data)", + "fingerprint": "b08def3e86ed6dc5cba388f51f1f206b_5", + "old_fingerprint": "e48a41fc1e83e126fc89b1f1f2a00793_5", + "code_extract": " $session->put($userInput, $data);" + } + ] +}" +`; + +exports[`php_symfony_session_key_using_user_input ok 1`] = `"{}"`; diff --git a/tests/php/symfony/session_key_using_user_input/test.js b/tests/php/symfony/session_key_using_user_input/test.js new file mode 100644 index 000000000..bca0fa61e --- /dev/null +++ b/tests/php/symfony/session_key_using_user_input/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/session_key_using_user_input/testdata/bad.php b/tests/php/symfony/session_key_using_user_input/testdata/bad.php new file mode 100644 index 000000000..b59ef1a82 --- /dev/null +++ b/tests/php/symfony/session_key_using_user_input/testdata/bad.php @@ -0,0 +1,36 @@ +getSession(); + $session->get($userInput, []); + $session->put($userInput, $data); + } + + public function someMethod(): void + { + $session = $this->requestStack->getSession(); + $session->get($userInput, []); + $session->put($userInput, $data); + } +} + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Annotation\Route; + +class FooController { + #[Route('/foo')] + public function foo(Request $request): Response { + $session = $request->getSession(); + $session->get($userInput, []); + $session->put($userInput, $data); + } +} diff --git a/tests/php/symfony/session_key_using_user_input/testdata/ok.php b/tests/php/symfony/session_key_using_user_input/testdata/ok.php new file mode 100644 index 000000000..e3eb5972a --- /dev/null +++ b/tests/php/symfony/session_key_using_user_input/testdata/ok.php @@ -0,0 +1,36 @@ +getSession(); + $session->get($ok, $userInput); + $session->put($ok, $userInput); + } + + public function someMethod(): void + { + $session = $this->requestStack->getSession(); + $session->get($ok, $userInput); + $session->put($ok, $userInput); + } +} + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Routing\Annotation\Route; + +class FooController { + #[Route('/foo')] + public function foo(Request $request): Response { + $session = $request->getSession(); + $session->get($ok, $userInput); + $session->put($ok, $userInput); + } +}