feat(scan): command injection, SSRF and traversal for Java; traversal and template escaping for Go (v0.7.0) - #91
Merged
Merged
Conversation
… and template escaping for Go
Release 0.7.0.
Command injection, SSRF and path traversal were implemented for JavaScript
and, in part, for Go, and never for Java. The same defect in the same
codebase was reported or not depending on which file it lived in — Java had
five rules to JavaScript's thirteen, and none of them covered a class this
common.
Java: Runtime.exec built by concatenation (CWE-78), outbound request to a
computed URL (CWE-918), file path built from request data (CWE-22), broken
cipher or ECB mode (CWE-327).
Go: file path built from request data (CWE-22), value marked as pre-escaped
HTML (CWE-79).
Deliberately not added: TLS verification, weak hashing and insecure
randomness. `tls-verification-disabled`, `weak-hash-on-credential` and
`insecure-randomness-for-secret` are language-agnostic and already cover
both languages, including Go's InsecureSkipVerify and Java's MessageDigest.
Duplicating them per-language would double-report.
Each rule is built against the corrected shape: the argv form of exec is not
matched, a constant URL is not matched, a canonical-path containment check
exonerates the traversal rules, AES/GCM is not matched, and
template.HTML on a literal is not matched. Bare Cipher.getInstance("AES")
is matched, because the JCE resolves it to AES/ECB/PKCS5Padding.
Verified against ionic-team/capacitor: 146 Java files, findings unchanged at
13 — no false positives. The two concatenated `new File` sites there build a
config path from a constant and are correctly left alone by needsContext.
debtap unchanged at 8. 111 tests, up from 105.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThreatCrush Security Scan156 finding(s) HIGH/CRITICAL: 14 | MEDIUM: 106 | LOW: 36
…and 106 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
…tures The self-scan flagged three lines of this file. All three are string literals holding sample code for another language — a PHP `eval` and a Java `Runtime.exec` — which the JavaScript rules match because the file they sit in is TypeScript. Same treatment as the existing fixtures: an inline disable naming the rule and why. The Java one is lifted into a variable first, so the suppression comment attaches to the line the finding is actually on rather than to the middle of a wrapped expression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Release 0.7.0.
The gap
Command injection, SSRF and path traversal were implemented for JavaScript and, in part, for Go — and never for Java. The same defect in the same codebase was reported or not depending on which file it lived in.
Rule counts before this PR:
Java and Go were the thin ones, and what was missing from them was not exotic — it was the OWASP top of the list.
Added
Java
java-runtime-exec-concatenationjava-ssrf-outbound-requestjava-request-path-traversaljava-broken-cipherGo
go-request-path-traversalgo-template-escaping-bypassDeliberately not added
TLS verification, weak hashing and insecure randomness.
tls-verification-disabled,weak-hash-on-credentialandinsecure-randomness-for-secretare language-agnostic and already cover both languages — the first already matches Go'sInsecureSkipVerify, the second already matches Java'sMessageDigest.getInstance("MD5"). Adding per-language copies would double-report the same line.Worth noting the existing pair are scoped to credential and secret context rather than firing on every MD5. That is the right call — MD5 as a cache key is not a vulnerability — and this PR does not disturb it.
Each rule is built against the corrected shape
exec(new String[]{"git", "checkout", branch})— the argv form — is not matched; only the single-string concatenation is.new URL("https://api.example.com/v1/status")is not matched; a computed URL is.getCanonicalPath().startsWith(base)check below the construction exonerates both traversal rules.Cipher.getInstance("AES/GCM/NoPadding")is not matched. BareCipher.getInstance("AES")is — the JCE resolves it toAES/ECB/PKCS5Padding, so the default is the mode the rule exists to catch.template.HTML("<br>")on a literal is not matched; a conversion of a variable is.Verification
ionic-team/capacitor: unchanged at 13 findings across 146 Java files. Zero false positives from six new rules.That number is load-bearing, so I checked what it was actually exercising rather than assuming absence of matching code:
Runtime.getRuntime()call sites — nothing for that rule to hit.Cipher.getInstancecall sites.new File(...)sites, bothnew File(path + "capacitor.config.json")— a constant filename on an internal config path.needsContextcorrectly holds them.new URL(variable)sites, all taking config or plugin-internal values that match none of Java's untrusted-input patterns. Correctly silent.So: no false positives, and the silences are explained rather than lucky. The unit tests carry the positive cases.
debtap: unchanged at 8. 111 tests, up from 105.
tsc --noEmitclean.