inspectFile: False-positive match fixed #1311
Conversation
The operator checks script exit code only and ignores the output. As the result, false-positive match occurs.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1311 +/- ##
=======================================
Coverage 81.77% 81.77%
=======================================
Files 170 170
Lines 9777 9777
=======================================
Hits 7995 7995
Misses 1533 1533
Partials 249 249
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| if ctx.Err() == context.DeadlineExceeded || err != nil { | ||
| return false | ||
| } | ||
| return true | ||
| return len(output) > 0 && output[0] != '1' |
There was a problem hiding this comment.
What is this condition about output[0] != '1'?
There was a problem hiding this comment.
It checks the first byte of the script output. Value 1 means that rule not matches. Any other value (e.g. 0) means that rule matches. I was unable to find any standard or specification of that "protocol". But there's an example script in modsec 2.x docs that prints 0 or 1 in depending on clamd scan result. Also, both modsec 2.x and 3.x implementations uses the first byte of script output, here are links: version 2.9.8 and version 3.0.13. Thus, my patch is an exact 1:1 port of C/C++ modsec 2/3 logic.
According to modsec docs, the operator should check script output as well as its exit code. Here is the implementation. But Coraza is missing that and ignores script output completely. I prepared a patch, please review.
p.s.: Unfortunately I don't have windows platform, so if somebody can help me to implement unit test for it, I would very appretiate that.