{"action":"created","issue":{"url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/380","repository_url":"https://api.github.com/repos/simplesamlphp/simplesamlphp","labels_url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/380/labels{/name}","comments_url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/380/comments","events_url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/380/events","html_url":"https://github.com/simplesamlphp/simplesamlphp/pull/380","id":152047815,"number":380,"title":"Allow regular expression matching of trusted.url.domains.","user":{"login":"jdufresne","id":347634,"avatar_url":"https://avatars.githubusercontent.com/u/347634?v=3","gravatar_id":"","url":"https://api.github.com/users/jdufresne","html_url":"https://github.com/jdufresne","followers_url":"https://api.github.com/users/jdufresne/followers","following_url":"https://api.github.com/users/jdufresne/following{/other_user}","gists_url":"https://api.github.com/users/jdufresne/gists{/gist_id}","starred_url":"https://api.github.com/users/jdufresne/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jdufresne/subscriptions","organizations_url":"https://api.github.com/users/jdufresne/orgs","repos_url":"https://api.github.com/users/jdufresne/repos","events_url":"https://api.github.com/users/jdufresne/events{/privacy}","received_events_url":"https://api.github.com/users/jdufresne/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2016-04-30T19:28:15Z","updated_at":"2016-06-01T13:07:32Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/pulls/380","html_url":"https://github.com/simplesamlphp/simplesamlphp/pull/380","diff_url":"https://github.com/simplesamlphp/simplesamlphp/pull/380.diff","patch_url":"https://github.com/simplesamlphp/simplesamlphp/pull/380.patch"},"body":"Fixes #379"},"comment":{"url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/comments/222986408","html_url":"https://github.com/simplesamlphp/simplesamlphp/pull/380#issuecomment-222986408","issue_url":"https://api.github.com/repos/simplesamlphp/simplesamlphp/issues/380","id":222986408,"user":{"login":"jaimeperez","id":1942728,"avatar_url":"https://avatars.githubusercontent.com/u/1942728?v=3","gravatar_id":"","url":"https://api.github.com/users/jaimeperez","html_url":"https://github.com/jaimeperez","followers_url":"https://api.github.com/users/jaimeperez/followers","following_url":"https://api.github.com/users/jaimeperez/following{/other_user}","gists_url":"https://api.github.com/users/jaimeperez/gists{/gist_id}","starred_url":"https://api.github.com/users/jaimeperez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jaimeperez/subscriptions","organizations_url":"https://api.github.com/users/jaimeperez/orgs","repos_url":"https://api.github.com/users/jaimeperez/repos","events_url":"https://api.github.com/users/jaimeperez/events{/privacy}","received_events_url":"https://api.github.com/users/jaimeperez/received_events","type":"User","site_admin":false},"created_at":"2016-06-01T13:07:32Z","updated_at":"2016-06-01T13:07:32Z","body":"Hi Jon! Thanks a lot for this! It's not that often that we get PRs so complete, with documentation and tests 😄 \r\n\r\nThere's one thing I've spotted in your code, though. If an attacker notices that your SimpleSAMLphp installation is fine to redirect to *app.com* (that is, *app.com* is allowed in `trusted.url.domains`), he or she can leverage that knowledge to circumvent the protection mechanism and exploit an open redirect. Let's say this is your configuration:\r\n\r\n```php\r\n    'trusted.url.regex' => true,\r\n    'trusted.url.domains' => array('app.com'),\r\n```\r\n\r\nThen the attacker could use a domain under his/her control, say *evil.com*, and register a subdomain *app.com.evil.com*, configuring a web server there to redirect to the final destination of the open redirect attack. Then, passing SimpleSAMLphp a URL such as `http://app.com.evil.com/some_URL_ID` would allow the attacker to skip the protection and get the end user redirected to somewhere else without noticing, because *app.com.evil.com* will match the regular expression that you are building, `app.com`, which is lacking regex delimiters as well as begin and end of line delimiters.\r\n\r\nThe proper way to handle this case would be something like this:\r\n\r\n```php\r\n               foreach ($trustedSites as $regex) {\r\n                    if (preg_match('@^'.$regex.'$@', $hostname)) {\r\n                        $trusted = true;\r\n                        break;\r\n                    }\r\n                }\r\n```\r\n\r\nI think we would need a test for this corner case too. Would you be ok to add that couple of changes to the PR?\r\n\r\nThanks again!"}}
{"actor":{"display_login":"jaimeperez"}}