Skip to content

Commit

Permalink
feat: add symfony open redirect rule
Browse files Browse the repository at this point in the history
  • Loading branch information
didroe committed Oct 9, 2023
1 parent 73c01e5 commit b91f8ac
Show file tree
Hide file tree
Showing 7 changed files with 193 additions and 1 deletion.
8 changes: 7 additions & 1 deletion rules/php/shared/lang/user_input.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
type: shared
languages:
- php
imports:
- php_shared_symfony_user_input
sanitizer: php_shared_lang_user_input_sanitized
patterns:
- pattern: $$<METHOD>[$<_>]
Expand All @@ -9,7 +11,11 @@ patterns:
values:
- _GET
- _POST

- pattern: $<SYMFONY_USER_INPUT>;
filters:
- variable: SYMFONY_USER_INPUT
detection: php_shared_symfony_user_input
scope: cursor_strict
auxiliary:
- id: php_shared_lang_user_input_sanitized
patterns:
Expand Down
14 changes: 14 additions & 0 deletions rules/php/shared/symfony/user_input.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
type: shared
languages:
- php
patterns:
- pattern: |
class $<CLASS_NAME> {
public function $<_>($<...>$<!>$<_>$<...>) {}
}
filters:
- variable: CLASS_NAME
regex: Controller\z
metadata:
description: "PHP user input."
id: php_shared_symfony_user_input
42 changes: 42 additions & 0 deletions rules/php/symfony/open_redirect.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
imports:
- php_shared_lang_user_input
patterns:
- pattern: $this->$<METHOD>($<USER_INPUT>$<...>)
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
78 changes: 78 additions & 0 deletions tests/php/symfony/open_redirect/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
@@ -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`] = `"{}"`;
16 changes: 16 additions & 0 deletions tests/php/symfony/open_redirect/test.js
Original file line number Diff line number Diff line change
@@ -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()
})
})
18 changes: 18 additions & 0 deletions tests/php/symfony/open_redirect/testdata/bad.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;

class FooController {
#[Route('/foo/{x}')]
public function foo(int $x): Response {
return $this->redirect($x);
}

#[Route('/bar')]
public function bar(): Response {
return $this->redirectToRoute($_GET["oops"], []);
}
}
18 changes: 18 additions & 0 deletions tests/php/symfony/open_redirect/testdata/ok.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace App\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;

class FooController {
#[Route('/foo/{x}')]
public function foo(int $x): Response {
return $this->redirect($ok);
}

#[Route('/bar')]
public function bar(): Response {
return $this->redirectToRoute($ok, []);
}
}

0 comments on commit b91f8ac

Please sign in to comment.