fix: preserve value in t:base64Decode when input is not valid base64

[0xst3m]

What

When t:base64Decode encounters non-base64 input, the transformation now preserves the original value unchanged and returns false (no change), instead of silently replacing it with an empty string and returning true.

Why

The base64Helper() function in src/utils/base64.cc calls mbedtls_base64_decode but ignores its error code return value. It only checks the out_len output parameter, which stays 0 when mbedtls returns MBEDTLS_ERR_BASE64_INVALID_CHARACTER (-0x002C). This causes Base64Decode::transform() to unconditionally replace the variable's value with an empty string for any non-base64 input.

Impact:

• All transformations after t:base64Decode in a pipeline run on an empty string — effectively dead code for non-base64 input
• CRS rules 934100, 934101, 934160 have degraded post-decode transformation pipelines
• CRS was forced to remove t:base64Decode from rules 934130/934131 as a workaround (coreruleset/coreruleset#3376)

How

Added Base64::tryDecode() which checks the mbedtls_base64_decode return value before proceeding:

• Returns MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL → valid base64, proceed with decode
• Returns anything else → not valid base64, return false (value preserved)

This is a whitelist approach — only the known-good return code proceeds. Any current or future error codes from mbedtls (including the new MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED in mbedtls v4.x) are automatically handled.

Files changed:

File	Change
src/utils/base64.h	Added tryDecode() method declaration
src/utils/base64.cc	Added tryDecode() implementation
src/actions/transformations/base64_decode.cc	Use tryDecode(), preserve value on failure

Backward compatibility:

• The existing Base64::decode() API is unchanged — remote_user.cc is not affected
• Compatible with both mbedtls v3.6.x and v4.x (tested against both)

References

• Closes Base64 decoding generates no warning #1037 (base64 decoding generates no warning — open since 2015)
• Closes base64decode behaviour  #3103 (base64decode behaviour — value destroyed on non-base64 input)
• Related: base64Decode behaviour against payload which contain + and / #3327 (base64Decode behaviour against payload with + and /)
• CRS workaround: coreruleset/coreruleset#3376
• CRS affected rules: 934100, 934101, 934160 (Node.js injection/DoS detection)

Proposed test additions

For secrules-language-tests transformations/base64Decode.json:

{
   "ret" : 0,
   "input" : "not;valid;base64",
   "type" : "tfn",
   "name" : "base64Decode",
   "output" : "not;valid;base64"
},
{
   "ret" : 0,
   "input" : "hello world!",
   "type" : "tfn",
   "name" : "base64Decode",
   "output" : "hello world!"
}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[fzipi]

Thanks for the fix — the core logic (checking the mbedtls return code) is correct and the bug analysis is solid.

One suggestion: rather than adding a new tryDecode() method alongside the broken decode(), could we just fix decode() itself?

The only other caller is remote_user.cc, which decodes the HTTP Basic Auth Authorization header — always valid base64 set by the HTTP server. Even there, silently returning an empty string on invalid input is wrong; it's just less likely to trigger.

Leaving the broken decode() around is a footgun for future callers. A cleaner approach would be:

1. Change decode() signature to return success/failure (e.g. bool decode(const std::string &data, std::string &out) or std::optional<std::string>)
2. Update both base64_decode.cc and remote_user.cc callers
3. Drop tryDecode() entirely

This keeps the API surface smaller and ensures all base64 decoding in the project handles errors properly.