Use WASM module of zen-internals instead of FFI - #347
Conversation
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
3a69bc3 to
62f342b
Compare
| instance = current.pool.acquire(); | ||
| } catch (Throwable e) { | ||
| logger.trace(e); | ||
| return false; |
There was a problem hiding this comment.
The catch-all failure path returns false from detectSqlInjection, treating WASM errors as safe input and bypassing SQL injection validation.
Details
✨ AI Reasoning
The new error path converts any WASM execution failure into a normal negative detection result. This makes failures indistinguishable from safe input and can disable SQL injection protection without surfacing the problem to callers.
🔧 How do I fix it?
Remove debugging statements like console.log, debugger, dd(), or logic bypasses like || true. Keep legitimate logging for monitoring and error handling.
Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info
Similar to go agent, packages the wasm module inside the agent jar and gets rid of the required native flag to disable warning In terms of perf, it's 3-4x faster than the current FFI approach. However, we're loading the FFI library on every invocation so it was slow by design. Caching the FFI library is much faster than the WASM approach but in a real application you don't notice the difference between cached FFI and WASM (because there's other overhead).
62f342b to
1ad6bb1
Compare
curl failures should stop verification
| // If zen-internals is unavailable, let the database query proceed. | ||
| return false; |
There was a problem hiding this comment.
🟠 High - WASM failures are handled fail-open, disabling SQL injection detection
detectSqlInjection catches initialization and instance-acquisition errors and returns false, which is the same result used for a clean query. Because SqlDetector treats false as "no attack" and Agent ignores the result of the startup initialize() call, a missing, corrupt, or otherwise broken zen-internals module silently turns off SQL injection protection instead of surfacing a hard failure. An attacker only needs the detector to be unavailable for malicious queries to be allowed through unchecked.
Show fix
Fail closed when the WASM detector cannot be initialized or executed. Either abort agent startup if initialize() fails, or make detectSqlInjection propagate a distinct failure so callers can block/disable the protected operation instead of interpreting detector errors as "no injection detected."
More info - Reply on this comment to give feedback or ignore the issue.
2 participants
Similar to go agent, packages the wasm module inside the agent jar and gets rid of the required native flag to disable warning
In terms of perf, it's 3-4x faster than the current FFI approach. However, we're loading the FFI library on every invocation so it was slow by design. Caching the FFI library is much faster than the WASM approach but in a real application you don't notice the difference between cached FFI and WASM (because there's other overhead).