Security: Fix CWE-78 (Command Injection) vulnerability in src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01942.java:63#662
Open
appsecai-app[bot] wants to merge 1 commit intomainfrom
Conversation
Add allowlist validation for HTTP header input before Runtime.exec() call. Input is now restricted to alphanumeric characters, spaces, and safe punctuation ([a-zA-Z0-9 @._-]). Invalid input is rejected with HTTP error. Fixes: 69c734eb (CWE-78 OS Command Injection)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What we found
69c734ebsrc/main/java/org/owasp/benchmark/testcode/BenchmarkTest01942.java:63Description: User-controlled input from an HTTP request header flows directly into a command array passed to
Runtime.exec(). While the array form ofexec()prevents shell interpretation, unvalidated input can still inject additional arguments or pass malicious data to the executed command.Why this matters
Risk if not fixed: An attacker could execute arbitrary operating system commands on the server by injecting additional process arguments, potentially gaining unauthorized access to system resources, reading sensitive files, or pivoting to other systems.
Attack surface: Any HTTP client can supply a malicious value in the
BenchmarkTest01942header. The header is URL-decoded before use, which expands encoded metacharacters and increases exploitability. No authentication or authorization check protects this endpoint.Why this is exploitable: The
doSomething()method returns the raw HTTP header value unchanged. This value is then embedded into a command array passed toRuntime.exec(). Although the array form ofexec()avoids shell metacharacter interpretation, an attacker can still inject additional process arguments or pass specially crafted values that the target command will interpret as instructions.Why we're changing it
Vulnerability confirmed: The data flow is direct and unobstructed:
param = request.getHeader("BenchmarkTest01942")— attacker controls this value via HTTP headerparam = java.net.URLDecoder.decode(param, "UTF-8")— URL-encoded characters are decoded, expanding any encoded payloadsdoSomething()returnsparamasbarwith zero transformation or validationr.exec(cmdArray, argsEnv, ...)—baris embedded into the command array without sanitizationNo allowlist, regex validation, or sanitization is present at any point in the data flow from source to sink.
How we confirmed
Vulnerability Flow Diagram
%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%% flowchart TD A["HTTP Header - BenchmarkTest01942"] --> B["URL Decode"] B --> C["doSomething() returns param unchanged"] C --> D["Embedded in cmdArray"] D --> E{"Input validation?"} E -->|No| F["❌ Runtime.exec with attacker-controlled argument"] E -->|Yes| G["✅ Rejected or safely executed"] style A fill:#EDE9FE,stroke:#7C3AED style B fill:#EDE9FE,stroke:#7C3AED style C fill:#FFE5E5,stroke:#F65A5A style D fill:#FFE5E5,stroke:#F65A5A style E fill:#FEF3C7,stroke:#F59E0B style F fill:#FEF3C7,stroke:#F59E0B style G fill:#DCFCE7,stroke:#16A34AManual Verification Steps
src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01942.javaat line 63bar(the HTTP header value) is embedded intocmdArraywithout prior validationdoSomething()method performs no transformation on the input parameterexec()callRunnable Verification Script (click to expand)
Save this script and run with
bash verify_fix.sh:Vulnerable flow: src/main/java/org/owasp/benchmark/testcode/BenchmarkTest01942.java:63
Command Injection
%%{init: {'theme':'base','themeVariables':{'fontFamily':'ui-sans-serif, Inter, system-ui, sans-serif','primaryColor':'#EDE9FE','primaryTextColor':'#1A1A2E','primaryBorderColor':'#7C3AED','lineColor':'#5B21B6','secondaryColor':'#FEF3C7','tertiaryColor':'#DCFCE7'}}}%% flowchart TD subgraph Vulnerable["❌ Vulnerable Flow"] direction LR A1["Project"] --> A2["OS command injection via user input"] A2 --> A3["💥 OS Command Executed"] end Vulnerable ~~~ Fixed subgraph Fixed["✅ Fixed Flow"] direction LR B1["Project"] --> B2["Command allowlist or subprocess array"] B3["🛡️ Attack Blocked"] B2 --> B3 end style A2 fill:#FFE5E5,color:#000 style A3 fill:#ffa94d,color:#000 style B2 fill:#74c0fc,color:#000 style B3 fill:#DCFCE7,color:#000How we fixed it
Root Cause
The
doSomething()method returns the raw HTTP header value unchanged. This value is then embedded into a command array passed toRuntime.exec(). Although the array form ofexec()prevents shell metacharacter interpretation, the unvalidated input can still be used to inject additional process arguments or pass malicious data to the target command.Fix Approach
An allowlist regex validation gate is inserted before command construction. Input that does not consist solely of alphanumeric characters, spaces, and a narrow set of safe punctuation (
[a-zA-Z0-9 @._\-]) is rejected with an HTTP error response. This prevents any attacker-controlled metacharacters, path separators, or shell special characters from reaching theexec()call.Why This Approach
Alternatives considered:
Runtime.exec()array form already avoids shell interpretation; the remaining risk is argument injection, which requires input validation regardless of the execution method.Why allowlist validation is the right choice:
Vulnerabilities Addressed
CWE-78
Tainted Cmd From Http Request
How we validated it
|,;,$, backticks, newlines, path separators, etc.) are now rejected rather than executed.How to verify
Reviewers can verify the fix by:
exec()callhello world→ should executehello; rm -rf /→ should be rejectedhello$(whoami)→ should be rejectedBefore you merge
Learn more
This fix was generated by AppSecAI. Please review before merging.