fix: makes uri decode platform independent

[M4tteoP]

REQUEST_URI variable content goes automatically through a URL decoding transformation process: https://github.com/SpiderLabs/ModSecurity/blob/5b094c0ce9044044f740e135df2a60c5f0858d4d/src/transaction.cc#L466 Internally the transformation is based on the HEX2DEC array mapping. https://github.com/SpiderLabs/ModSecurity/blob/5b094c0ce9044044f740e135df2a60c5f0858d4d/src/utils/string.h#L34-L55 HEX2DEC array returns (char)-1 when the conversion fails. The result is then compared with -1:

https://github.com/SpiderLabs/ModSecurity/blob/5b094c0ce9044044f740e135df2a60c5f0858d4d/src/utils/decode.cc#L102-L103

The C/C++ standard is not strict about the char implementation, allowing it to be signed or unsigned. Because of that, if an environment interprets the char as an unsigned char the conversion is never considered as failed ((char)-1 overflows to 255 and is compared against -1).

This PR proposes to make the comparison more portable, forcing the comparison between a platform-dependant value ((char)-1).

Discovered running the owasp/modsecurity-crs:nginx image on Apple silicon processors. Example Rule that is failing:

SecRule REQUEST_URI "@rx \x25" \
    "id:920220,\
    phase:1,\
    deny,\
    t:none"

Request:

curl -v "http://localhost:80/?x=%w20"

%w should not be interpreted as a valid encoded sequence, therefore % should still be present in the REQUEST_URI and the regex should match.

Variables affected: REQUEST_URI, REQUEST_FILENAME Dug into it with: @theseion, @theMiddleBlue, @airween

[sonarqubecloud[bot]]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
2.6% Duplication on New Code

See analysis details on SonarCloud

[airween]

Hi @M4tteoP, thanks for this PR. I remember this issue, now I reviewed it again and I think both the explanation and solution is very nice. If there won't be any remarks, I will merge it soon.