feat(933100): add detection of smarty template php tag#4447
Conversation
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
|
📊 Quantitative test results for language: |
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
|
@azurit Can you review this one? Seems like you have more experience in PHP than myself 😄 |
There was a problem hiding this comment.
Pull request overview
This PR extends CRS rule 933100 (PHP open tag detection) to also detect Smarty-style {php} / {/php} template tags, and adds regression tests for additional PHP open-tag variants and non-matching cases.
Changes:
- Update rule 933100 regex to match
{php}-style tags in addition to existing<?...and[php]patterns. - Expand regression tests for 933100 with new positive and negative cases.
- Update the corresponding regex-assembly source for rule 933100.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
rules/REQUEST-933-APPLICATION-ATTACK-PHP.conf |
Extends rule 933100 regex to include curly-brace {php} tag detection. |
regex-assembly/933100.ra |
Updates regex-assembly definition intended to generate the new bracket/brace matching. |
tests/regression/tests/REQUEST-933-APPLICATION-ATTACK-PHP/933100.yaml |
Adds extensive positive/negative coverage for PHP open tags and {php} tags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@fzipi Thanks for your trust :) I’ve seen your change in the .ra file, which results in a mix of curly braces and square brackets that triggers Copilot warnings. However, I don’t see any case where this could actually have an impact, so we can leave it as is. As for all the unit tests you’re adding one after another ^^ are you using Claude for that? Maybe Copilot itself ? |
|
Rule is blocking also invalid patterns like |
|
@touchweb-vincent I'm using Claude code, with a skill. |
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
|
@azurit Thanks for that. Fixed. |
Proposed changes
Hello
It appears that nothing prevents PHP tags inserted into Smarty templates on versions 2 up to 3.1.20 - and probably other template engines as well.
https://www.smarty.net/docsv2/fr/language.function.php.tpl
No rule currently detects this in PL2.
curl -H "x-format-output: txt-matched-rules" -H "x-crs-paranoia-level:4" "http://sandbox.coreruleset.org/" -d "payload={php}test{/php}"I’m not aware of any modern CMS that uses this; however, that doesn’t mean none exist. What do you think? Should we leave it as is, or add support for it?
Thanks
PR Checklist
commentfield to write the expected behaviorFurther comments
For the reviewer
ctl:requestBodyAccess=Offwere used in the rule