{"action":"created","issue":{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/338","repository_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs","labels_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/338/labels{/name}","comments_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/338/comments","events_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/338/events","html_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/338","id":150681548,"number":338,"title":"PHP function name detection","user":{"login":"lifeforms","id":3464056,"avatar_url":"https://avatars.githubusercontent.com/u/3464056?v=3","gravatar_id":"","url":"https://api.github.com/users/lifeforms","html_url":"https://github.com/lifeforms","followers_url":"https://api.github.com/users/lifeforms/followers","following_url":"https://api.github.com/users/lifeforms/following{/other_user}","gists_url":"https://api.github.com/users/lifeforms/gists{/gist_id}","starred_url":"https://api.github.com/users/lifeforms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lifeforms/subscriptions","organizations_url":"https://api.github.com/users/lifeforms/orgs","repos_url":"https://api.github.com/users/lifeforms/repos","events_url":"https://api.github.com/users/lifeforms/events{/privacy}","received_events_url":"https://api.github.com/users/lifeforms/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/labels/V3.0.0-rc1%20PHP%20Project","name":"V3.0.0-rc1 PHP Project","color":"d4c5f9"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":5,"created_at":"2016-04-24T17:10:25Z","updated_at":"2016-05-11T15:55:57Z","closed_at":null,"body":"PHP function name detection is useful to detect PHP code injection. We have several issues open about this (#290, #291, #326), but I think they are best addressed at once.\r\n\r\nChallenges with detecting PHP functions are false positives (due to overlap with English words), false negatives (need to be strict on dangerous functions), and performance (there are many PHP functions).\r\n\r\n## Grouping the PHP functions\r\n\r\nI think the issue becomes most clear if we separate the PHP functions on two dimensions: the abuse potential, and the expected false positive rate. When graphing the space, you can think of something like this:\r\n\r\n![PHP functions](https://lf.ms/php-function-names.png)\r\n\r\n'Dangerous' high-abuse functions are often seen in actual code injection exploits. From my experience, they are centered around **decoding** (unpacking a compressed or obfuscated payload), **file/URL access** (for instance a remote payload or a local file) and **execution** (interpreting code or starting some other process).\r\n\r\nHowever, any other PHP function may still be used in a PHP payload, so it's useful to detect these as well. Many attackers encode their payload however, if not to evade WAFs then to comfortably inject code with newlines.\r\n\r\nWe must pay close attention to false positives. Many PHP function names are used in English words.\r\n\r\n## Which actions to take for each group\r\n\r\nI think we should handle every group separately. My current idea and rationale is as follows (but this is up for debate!)\r\n\r\n- ***Group I e.g. `sleep`, `time`, `abs`: Remove these functions from our data file and do not detect them.*** Detecting these words looks like more trouble than it is worth, since these functions are of low value to an attacker, but the false positives raise the cost for the user. We might lose some detection, but that's life. (PS: We could consider creating a rule for these at high paranoia level, or creating a regexp-based rule to lower FP, but frankly, I personally don't think some functions are worth the extra data file or maintenance).\r\n- ***Group II e.g. `strpos`, `setlocale`: Add to data file php-function-names.data. Do @pmf check on this file. At paranoia level PL1, log as anomaly. At PL2, block as critical.*** These functions are arguably used in real-life PHP code, so we should detect them. There are a LOT of functions in this category, so for performance reasons, we should do @pmf on such a big file just once. A case could be made for logging these as critical in PL1 as well; this would have the advantage that the rules will become less complicated.\r\n- ***Group III e.g. `system`, `exec`, `eval`: Create a regexp-based rule to look for these words plus a trailing parenthesis. At PL1, block as critical.*** These few functions are absolutely vital to block, and at the same time very FP prone. We can't simply block on a data file with @pmf, because we need regexp magic to ensure that we see real PHP function syntax, e.g. `eval(` and not `medieval`. There are not too many of these functions, so a regexp rule would be neither hard to maintain nor too slow in my opinion.\r\n- ***Group IV e.g. `gzinflate`, `base64_decode`: Add to data file php-function-names.data, but also add another rule that at PL1, blocks as critical.*** Since these strings are less common, we don't need a regexp to confirm function, and we can just block as we find them. Why not insert them into the regexp of Group III? Well, the regexp might be evaded by a clever ruse, so we should keep Group III as small as possible. If you see `base64_decode` it's bad news, don't look for the parenthesis and assign a very high score. Why have a duplicate rule since these terms are also in the data file? Because they should be blocked as (super)critical since they are highly specific to attacks; they should score (much) higher than ones in Group II. I'm tending towards a regexp rule for Group IV for maintenance reasons: Group III and IV will then use the same mechanism, and we'll just have one big data file for the 'low FP' functions. However, it's also fine to create a data file.\r\n\r\n## Example payloads\r\n\r\nThis approach has the advantage that there will be a diversity of rules, so a common PHP payload will trigger multiple rules and rack up an interesting score. For example, here are some random examples from audit logs today, to demonstrate in which way these functions are injected and chained by attackers. Especially `eval` is popular:\r\n\r\n- `eval(chr(112).chr(104).chr(112).chr(105).chr(110).chr(102).chr(111)...`\r\n- `eval(gzinflate(str_rot13(base64_decode('vUl6QttVE...`\r\n- `eval(base64_decode('JGNoZWNrID...`\r\n\r\n## Rules\r\n\r\nThe rules could work like this:\r\n\r\n1. Match on Group II file `@pmf php-function-names.data`, capture hit\r\n2. If PL1: log captured as anomaly\r\n3. If PL2+: log captured as critical\r\n4. Do regexp for Group III, log as critical\r\n5. Do regexp for Group IV, log as critical\r\n\r\nA work in progress rule for Group III could look like this:\r\n\r\n```\r\n# We want to match on:\r\n# system(\r\n# system (\r\n# system\\t(\r\n# system /*comment*/ (\r\n# system /*multiline \\n comment*/ (\r\n# system //comment \\n (\r\n# system #comment \\n (\r\n#\r\n# We don't want to match on:\r\n# the system is down\r\n# ecosystem(\r\n# systematic(\r\n#\r\n# ModSecurity does a multi-line regexp as of 2.9, so multiline comments are no problem.\r\nSecRule ARGS \"\\b(eval|system)(\\s|/\\*.*\\*/|//.*|#.*)*\\(\" \\\r\n\t\"id:123456,phase:1,t:none,t:urlDecodeUni,t:lowercase,deny,status:406,tag:'TEST/GROUP3'\"\r\n```\r\n\r\n## Todos\r\n\r\n- [ ] decide on approach\r\n- [ ] create 'master list' of functions from current CRSv3 data, Chaim's list in #290\r\n- [ ] filter high-FP functions from this list (go through master list manually, also find intersection with dictionary e.g. /usr/share/dict on FreeBSD)\r\n- [ ] filter high-abuse functions (obfuscation, access, execution; see audit logs)\r\n- [ ] separate all functions in groups\r\n- [ ] decide action for group I\r\n- [ ] decide action for group II\r\n- [ ] decide action for group III\r\n- [ ] decide action for group IV\r\n- [ ] create rules\r\n- [ ] test on audited attack traffic, common webshells, etc.\r\n- [ ] improve rules\r\n"},"comment":{"url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/comments/218504076","html_url":"https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/338#issuecomment-218504076","issue_url":"https://api.github.com/repos/SpiderLabs/owasp-modsecurity-crs/issues/338","id":218504076,"user":{"login":"lifeforms","id":3464056,"avatar_url":"https://avatars.githubusercontent.com/u/3464056?v=3","gravatar_id":"","url":"https://api.github.com/users/lifeforms","html_url":"https://github.com/lifeforms","followers_url":"https://api.github.com/users/lifeforms/followers","following_url":"https://api.github.com/users/lifeforms/following{/other_user}","gists_url":"https://api.github.com/users/lifeforms/gists{/gist_id}","starred_url":"https://api.github.com/users/lifeforms/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/lifeforms/subscriptions","organizations_url":"https://api.github.com/users/lifeforms/orgs","repos_url":"https://api.github.com/users/lifeforms/repos","events_url":"https://api.github.com/users/lifeforms/events{/privacy}","received_events_url":"https://api.github.com/users/lifeforms/received_events","type":"User","site_admin":false},"created_at":"2016-05-11T15:55:57Z","updated_at":"2016-05-11T15:55:57Z","body":"@dune73, sorry for taking so much time to respond.\r\n\r\nI appreciate that you are trying to keep the rules as simple as possible, although I'm not fully agreeing with the outcome yet, maybe we can find a compromise! :)\r\n\r\n- **Group I e.g. `sleep`, `time`, `abs`:** We both agree to ignore these functions because of FP risk, so unless @csanders-git thinks otherwise, we'll consider that **settled.**\r\n- **Group II e.g. `strpos`, `setlocale`:** I proposed *anomaly* severity at PL1 and a critical sibling at PL2, you prefer *critical* severity at PL2 or 3. The distinction between those two can be subtle. It's hard for me to say which is more useful. I thought this group would be nice as a signal in PL1. But if we let the severity depend on paranoia level, we introduce complexity, and I appreciate that you want to reduce this, so I agree to use a single rule. As for the paranoia level 2 or 3: we will curate Group II to have low FP anyway. I estimate that this rule will definitely have fewer FP than some of the XSS rules we brought back in PL2. So, let's pick PL2. So, I propose to take one of your options: **Make a single rule, at critical severity, at PL2, using @pmf.**\r\n- **Group III e.g. `system`, `exec`, `eval`:** I proposed *critical* severity at PL1 with a regexp to find adjoining `(` character. You proposed PL2 with a chained rule. As someone who protects PHP apps most of the time, I think this rule is highly necessary and should be enabled by default. A similar in-house rule gives me lots of matches from botnets attacking Wordpress plugins, and has never given me any FP. In my opinion, the lack of FP is almost certainly due to the regexp. If we'd just use a chained rule to check for `(` at any location in a matched variable, it will almost certainly raise FP, since the search terms themselves are - by design - very common - but the same goes for the use of parentheses. It's easy to think of legitimate plaintext containing sequence `(` as well as `chr` or `eval` somewhere. So, I am against the chained check, as I predict that people would hate the rule, or worse, disable it, while it could be very useful. I also think the complexity of the regexp is not too onerous for such a useful rule. My experience in production with it has been positive, and we'll document it with tests. So, I propose to **reconsider having this critical rule at PL1, using a regexp and not a chained rule.**\r\n- **Group IV e.g. `gzinflate`, `base64_decode`:** I proposed *critical* severity at PL1 with a regexp. You proposed the same but using @pmf. I agree with you; the regexp has little advantage here, so let's use @pmf. So I copy your proposal: **Run at critical severity, at PL1, using @pmf.**\r\n\r\nIf you think my Group III regexp proposal would be complex too maintain, I think the added tests will make it pretty easy to see how it works. Here are some example tests used to develop my group III regexp:\r\n\r\n```\r\n  - test: [{url: '/', code: 200}]\r\n  - test: [{url: '/?foo=system%28', code: 403}]\r\n  - test: [{url: '/?foo=System%28', code: 403}]\r\n  - test: [{url: '/?foo=system%0D%28', code: 403}]\r\n  - test: [{url: '/?foo=system%0A%28', code: 403}]\r\n  - test: [{url: '/?foo=system%0D%0A%28', code: 403}]\r\n  - test: [{url: '/?foo=system%20%28', code: 403}]\r\n  - test: [{url: '/?foo=system%20%20%28', code: 403}]\r\n  - test: [{url: '/?foo=%40system%28', code: 403}]                       # @system(\r\n  - test: [{url: '/?foo=system%09%28', code: 403}]                       # system\\t(\r\n  - test: [{url: '/?foo=system%2F%2Fcomment%0A%20%28', code: 403}]       # system//comment\\n (\r\n  - test: [{url: '/?foo=system%20%23%23comment%0A%20%28', code: 403}]    # system #comment\\n (\r\n  - test: [{url: '/?foo=system%20%23%23%20%28', code: 403}]              # system #\\n (\r\n  - test: [{url: '/?foo=system%2F%2Acomment%2A%2F%28', code: 403}]       # system/*comment*/(\r\n  - test: [{url: '/?foo=system%20%2F%2Acomment%2A%2F%20%28', code: 403}] # system /*comment*/ (\r\n  - test: [{url: '/?foo=system%20%09%2F%2A%2A%2F%09%20%28', code: 403}]  # system \\t/**/\\t (\r\n  - test: [{url: '/?foo=system%20%09%2F%2Amulti%0D%0Aline%2A%2F%09%20%28', code: 403}] # system \\t/*multi\\r\\nline*/\\t (\r\n  # prevent false positives:\r\n  - test: [{url: '/?foo=eval', code: 200}]\r\n  - test: [{url: '/?foo=cheval', code: 200}]\r\n  - test: [{url: '/?foo=the%20system', code: 200}]\r\n  - test: [{url: '/?foo=ecosystem%28', code: 200}]\r\n  - test: [{url: '/?foo=systems%28', code: 200}]\r\n  - test: [{url: '/?foo=system%20something%28', code: 200}]\r\n```"}}