test(core): Add reproducer for chain-pattern literal static-receiver mismatch#150
Draft
misonijnik wants to merge 3 commits into
Draft
test(core): Add reproducer for chain-pattern literal static-receiver mismatch#150misonijnik wants to merge 3 commits into
misonijnik wants to merge 3 commits into
Conversation
A multi-statement sink pattern that uses
`HttpRequest.newBuilder().uri($URL)` as a literal sub-expression in
the let-binding `$BUILDER = ...uri(...)` does NOT match a chained
source like
HttpRequest req = HttpRequest.newBuilder()
.uri(URI.create(t))
.GET()
.build();
client.send(req);
even though `HttpRequest.newBuilder()` is literally the receiver of
`.uri(...)` in the source. Replacing the literal static-method
receiver with `$_` (`$BUILDER = $_.uri($URL)`) matches the same
shape correctly — that's the workaround currently used in the
production `ssrf-sinks.yaml` `java-ssrf-sink` block (see the
`Builder-chain inline forms` comment). The literal form ought to
match the same source but doesn't.
Stubs (`issues/iChain/`) mirror the JDK `java.net.http.HttpRequest`
+ `java.net.URI` + `java.net.http.HttpClient` API surface needed
for the chain; the rule and test reference only the stub classes so
the reproducer doesn't depend on JDK classpath presence.
The test fails with:
Expected [PositiveCase(className=issues.issueChain$PositiveTaint)]
to be positive, but no vulnerability was found.
(constructor call `new okhttp3.Request.Builder().url(x)` works as a
literal in the same SSRF rule, so the mismatch is specific to a
static-method call in the receiver slot of the next chained call.)
…erpart Reframe the issueChain reproducer: the literal nested-receiver sink (`$BUILDER = newBuilder().uri($URL)` then `$BUILDER.build()`) not matching the fluent chain `newBuilder().uri(...).GET().build()` is INTENDED behaviour, not an analyzer false-negative. The calls are matched one at a time, so the order/structure of calls in the rule has to line up with the source; the intervening `.GET()` means `newBuilder().uri(...)` is never the direct receiver of `.build()`, so the nested literal receiver never binds. Drop the inaccurate docs that claimed a `$_.uri($URL)` form is "used in production ssrf-sinks.yaml (Builder-chain inline forms)" — that block does not exist in ssrf-sinks.yaml. Add issueChainSplitBuilder: same source, but bind `newBuilder()` in its own pattern-inside (`$NEW_BUILDER = ...newBuilder();`) and then match `$BUILDER = $NEW_BUILDER.uri($URL)`, matching the chain call-by-call. This fires (test passes), showing the correct way to express the intent. The issueChain test stays enabled/red as a documented reproducer.
The issueChain sample's tainted fluent-chain case was named
PositiveTaint,
so the sample harness (SampleData/SampleBasedTest) expected a finding.
The
chain-pattern rule legitimately cannot match that shape — the nested
static
receiver never binds (see the class Javadoc) — so the `issue
chain-pattern
literal static receiver` test was deliberately failing ("red").
Reclassify it as an expected non-detection by renaming the class to
NegativeTaintIntendedNonMatch (distinct from the existing literal-only
NegativeTaint case), which the harness treats as no-finding-expected.
The
test now passes. Update the now-inaccurate "expected to be red" comments
in
issueChain.java and IssuesTest.kt to match.
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
2 participants
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.
A multi-statement sink pattern that uses
HttpRequest.newBuilder().uri($URL)as a literal sub-expression in the let-binding$BUILDER = ...uri(...)does NOT match a chained source likeeven though
HttpRequest.newBuilder()is literally the receiver of.uri(...)in the source. Replacing the literal static-method receiver with$_($BUILDER = $_.uri($URL)) matches the same shape correctly — that's the workaround currently used in the productionssrf-sinks.yamljava-ssrf-sinkblock (see theBuilder-chain inline formscomment). The literal form ought to match the same source but doesn't.Stubs (
issues/iChain/) mirror the JDKjava.net.http.HttpRequestjava.net.URI+java.net.http.HttpClientAPI surface needed for the chain; the rule and test reference only the stub classes so the reproducer doesn't depend on JDK classpath presence.The test fails with:
Expected [PositiveCase(className=issues.issueChain$PositiveTaint)]
to be positive, but no vulnerability was found.
(constructor call
new okhttp3.Request.Builder().url(x)works as a literal in the same SSRF rule, so the mismatch is specific to a static-method call in the receiver slot of the next chained call.)