feat(model): Make passthrough models more precise by misonijnik · Pull Request #315 · seqra/opentaint

misonijnik · 2026-07-29T16:12:50Z

No description provided.

The shipped rule-tests exercise the sink and source rules, not the library models behind them, so a passthrough slot could be renamed, re-typed or dropped without a single test moving. These samples close that gap: each unsafe method walks one modelled call chain from a request parameter into Runtime.exec (or a file sink), and its safe twin walks the identical chain over a constant, so a broken model shows up as a false negative and an over-broad one as a false positive. Covered: AbstractStringBuilder#content (String and char[] overloads), StringBuffer#insert, String#format's boxed varargs element, StringJoiner (element and delimiter), Matcher#input via group() and appendTail(), MessageFormat#pattern, ByteArrayOutputStream#buffer, Reader#content, ByteBuffer/CharBuffer, BasicControl#oid, Rdn#type, SortKey#matchingRuleId, Reference#className, HttpHeaders#headerMap, the java.io.File path slot across seven constructor/accessor pairs, and the eleven explicit HttpServletRequest accessor models that replaced the engine's implicit get* passthrough. Two of these started red - Reader#read(char[]) and the nio flip() chain - and are green as of the model fixes in this batch.

2633 was measured at 2e67c6f, six commits before ae9d09995 restored the passthrough rules the batch had deleted, and was never re-measured afterwards. The gate as committed fails. Measured locally on one BenchmarkJava checkout with two analyzer jars built from the same rules and differing only in model/: 3-rules TraceGenerationStats(total=4338, simple=503, generatedSuccess=3835) 4-config TraceGenerationStats(total=4338, simple=503, generatedSuccess=3835) The batch is trace-neutral, so the expectation goes back to the 4112 that 3-rules gates on upstream BenchmarkJava (the local corpus is the explyt fork, which runs +226 over upstream - only the delta transfers, and the delta is zero).

A slot is identified by the whole triple (className, fieldName, fieldType), and the value type also becomes the base type for whatever hangs off the slot (JIRFactTypeChecker.accessorActualType). java.lang.Object is the one spelling that is never rejected: a base mark copied into the slot always survives, an element or field accessor may still hang off it, and the write side and the read side cannot drift into two different slots the way twenty of them did in this batch. 1867 positions across 50 files; the only exception is java.lang.String#<string-bytes>#byte[], which the engine constructs with that exact triple (TaintEvaluator.kt:72) and the config has to keep spelling the same way. Supersedes the direction of 3c7a4d64b, which unified the twenty split slots onto their precise declared types instead. 35 copies became duplicates of an existing edge once the types matched and were dropped; verified edge-preserving per entry.

…ound Three models were missing an edge that the container/serialization regression samples exercise: String#join(CharSequence, Iterable) element -> result was missing, so the String#join(CharSequence, CharSequence[]) joined elements never reached the result; arg(*) -> result only carries a whole-object mark, and an element mark re-roots as result.Element, which no scalar sink sees. Properties#store/storeToXML/load/loadFromXML not modelled at all, only #list. Yaml#dump/dumpAs/dumpAsMap/dumpAll/serialize only the load half existed; the dump half now writes the same <serialized-value> slot the load half reads, so a dump/load round trip on one Yaml instance is bridged. Samples: security.passthrough.PassthroughContainerSamples (arrays incl. copyOf and System.arraycopy, List get/iterate/copy/unmodifiable/toArray/join/stream, LinkedList, Set, Map value/key/values/entrySet, Properties) and PassthroughSerializationSamples (jackson write/read/read-bytes/readTree/convertValue, snakeyaml dump/load, Properties store), each unsafe method paired with a constant-fed safe twin. Two cases are deliberately not asserted, both structural rather than model bugs: serialising a bean whose *field* is tainted (the mark lands on json.name, not on json, and collapsing it needs an AnyField read), and ObjectOutputStream writing through to the ByteArrayOutputStream it wraps (aliasing, not a passthrough edge).

…ainers The leading-dot signature fix revived seven models that copy into a caller-supplied array rather than into the result, and nothing pinned them. Added: String#getChars, ByteBuffer#get(byte[]), CharBuffer#get(char[]) and a servlet doing getInputStream().read(byte[]). Container coverage extended to the bulk and nested moves: Map#getOrDefault, Map<String,List<String>> two hops deep, List#addAll, Collections#addAll, Map#putAll, Collectors#joining and Deque#push/poll.

The collector collapses the stream elements into one String, and the model would have to know which collector reached collect(..) to express that. Modelling it at the Stream#collect level instead would put an element mark on the root of every collected container, which is the kind of over-approximation this batch is trying to remove, so the sample stays as documentation and the gap stays open.

ae9d09995 put back every edge the batch had deleted, on the theory that a deletion is a lost rule. Most of those edges are the whole-object twin the batch had deliberately replaced with a field-sensitive slot, so restoring them re-created the imprecision the batch set out to remove - and it is what pushed the OWASP trace count back up, which is why EXPECTED_TRACES had to be raised. Measured on one BenchmarkJava checkout, analyzer jars differing only in model/: before the restore (2e67c6f) traces 2859 TP 1286 category-FP 559 off-category 399 after the restore (ae9d09995) traces 4338 TP 1286 category-FP 559 off-category 1385 this commit traces 2859 TP 1286 category-FP 559 off-category 399 The restore buys no true positive - TP is identical at 1286 across every state - and costs ~986 extra findings on test cases whose vulnerability is of another category, i.e. noise. Dropping it puts the trace count back on the 2633 the branch gates on (2859 here; the local corpus is the explyt fork, which runs +226 over upstream). 1910 edges removed by shape: 656 whole->slot, 599 slot->whole, 427 whole->whole, 142 element->slot, 60 element->element (the Go slices folds, redundant because the whole copy already carries element to element), 26 others. 218 entries were left with an empty copy list and removed with them. The rule-tests are the guard on the other side: 677 pass with 0 false negatives, including the 130 new passthrough samples added in this batch, so no modelled flow in the suite depended on a restored edge.

Measured after dropping the blanket restore: 2859 traces on the local fork, which is 2633 on the upstream corpus CI checks out - the number the batch set at 2e67c6f. Reverts 6d425e62f, which raised it to 4112 on the evidence that the batch as committed produced 4338 locally. That measurement was right about the committed tree and wrong about what to do with it: the fix was to take the restore back out, not to accept its trace count.

Written test-first: PassthroughValueFlowSamples#stringFormatLocaleUnsafe and PassthroughFileModelSamples#fileSystemGetPathUnsafe both failed before this change and pass after it. A whole copy X -> Y re-roots X's subtree at Y, so X[i] lands as Y[i]; the type checker keeps that path only where Y may hold elements. Where the destination is a scalar the element facts are dropped unless an explicit [X,'[*]'] -> Y copy carries them. Auditing every (function, signature) group for an array-typed source flowing into a scalar destination with no element edge anywhere in the group found 21 such holes: String#format(Locale, String, Object[]) the varargs box - the plain (String, Object[]) overload has the carrier, the Locale one did not FileSystem#getPath(String, String[]) every trailing path segment String#copyValueOf(char[]) and (char[],int,int) Spliterators#spliterator - eight array overloads (Arrays#spliterator already had it) JsonGenerator#writeBinary, ObjectBuffer#completeAndClearBuffer, String#charAt kotlin StringsKt append(Appendable|StringBuilder, array) Also pinned on the precision side, all three passing as negatives: a mark on a primitive result (String#length), a map value reaching the key side, and a map key reaching the value side. Checked and deliberately not touched: 127 copies whose destination is a primitive are not dead - a mark on a primitive is accepted when it carries the primitive-tracking mode, which rules opt into with `primitive-tracking: true`.

CallPositionToJIRValueResolver resolves an Argument position with callExpr.args.getOrNull(index), so a copy naming arg(0) on a zero-argument overload - JspWriter#println(), SelectItemGroup#<init>(), JsonGenerator#writeStartObject() and friends - resolves to nothing and can never fire. They come from per-overload signature splits that left the whole copy list on every overload. Deadness is decided from the entry's own signature, never from a library jar, and only where that signature is the exact string form: SerializedSignatureMatcher.Simple enforces `method.parameters.size == args.size`, so `signature: () void` binds to zero-argument methods whatever version of the library is on the classpath. A partial `{params: [{index, type}]}` matcher constrains only the listed indices and says nothing about the total arity, so a higher arg(N) there is NOT provably dead. Three such entries are deliberately untouched - AxiomSoapMessage#<init> really does take (SOAPMessage, Attachments, SOAPFactory, ..), and oro's Util#split / Util#substitute really do take a trailing int. 61 copies removed across 5 files, each verified out of range against its own exact signature; entries left with an empty copy list go with them. Rule-tests unchanged at 685 pass / 0 FN / 0 FP.

A String is immutable and scalar, so its taint belongs at the String itself, not in a bytes/chars/content sub-slot. Two things follow from the sub-slot that this removes: - It is invisible where it matters. A plain ContainsMark sink reads the mark *at* the position, so an edge like `-> result.java.lang.String#content` never reached one; the whole-object twin sitting next to it was doing all the work. - It is what forces the engine to special-case cleaning. A depth-one sanitizer clean clears the String but not `str.bytes`, so the next getBytes() reads the taint straight back out - which is why JIRTaintCleanActionEvaluator carries a hardcoded String bytes accessor and a `todo: fix in config?`. With the slots gone that special case has nothing left to clean; removing it belongs to the engine branch. 63 modifiers flattened onto their position, across the String constructors, getBytes/getChars/toCharArray/copyValueOf/valueOf/subSequence, Matcher#group/ replaceAll/toString, Base64$Decoder#decode, StringJoiner#toString, Normalizer#normalize, CachedRowSet, TextMessage/BinaryMessage#toStringPayload, ByteArrayResource and ExtendedRequest#getID. Most collapse into the whole-object twin that was already there. Nested slots (result.CharBuffer#value.String#chars) stay - no clean ever targets those. Pinned from both sides: SsrfSamples$UnsafeBytesRoundTripProxyServlet - a new String(byte[]) / getBytes round trip must still carry taint - and XssServletSamples$SafeGreetingBytesRoundTripServlet - the same round trip after a sanitizer must stay clean.

Measured against ThingsBoard, a Spring Boot 3 project: 40 reference-returning servlet accessors had no model at all, and the same 41 were missing on the javax side, so this is a coverage hole in the servlet model rather than a jakarta parity bug. What covered them instead was the engine's default get model, which copies the whole `this.<get-default>` subtree onto the result - it cannot tell "reads request data" from "returns the container", so `getServletContext()` inherited the request's entire mark tree exactly like `getAttribute()` did. Two kinds of entry, on both APIs: - Attribute store/read pairs on ServletRequest and HttpSession. setAttribute writes what getAttribute reads back, through named `attributes` and `attribute-names` slots. The session pair earns its place on its own: the trust-boundary rule deliberately excludes getSession from being a source, since the session is trusted-side state, so the only thing that can make a session read tainted is an earlier store - which is precisely what the slot models and what nothing modelled before. - Client-influenced request metadata: scheme, server name, remote address/host, protocol, content type, character encoding, context path and method, each from its own slot. Nothing writes these, so they carry a mark only from a starred request source, and then as a single leaf rather than as the request's whole tree. The jakarta file additionally receives the 21 entries that existed only on javax - Cookie, PushBuilder, ServletRequestWrapper#<init> and the two ServletResponse output accessors - each checked against the jakarta 6.0 API before being modelled. Both files now hold 59 entries and are namespace-symmetric. Pinned from both sides: RequestAttributeRoundTripServlet and SessionAttributeRoundTripServlet as positives, and SafeAccessorsServlet extended to exercise both attribute calls with constants, so the slots may not leak into an unrelated read. Rule tests 689 pass / 0 FN / 0 FP / 0 skipped (was 687). OWASP unchanged at 2859 traces locally, TP 1286 / FP 559 / off-category 399 - on this branch the default get model still fires unconditionally, so these edges are additive here; they only replace it on 4.5, where the model is gated on no rule having matched.

java.util.List#get was the one unmodelled method carrying taint through the default get model on conductor: it accounted for 1047 of the 1053 facts that model produced, across only three methods. The default get model reads a virtual this.<get-default> field that nothing writes, so it only matches a receiver whose node is abstract - it is the fallback that fires exactly where precision was already lost. Reading the element slot instead states the real flow: an element goes in through the collection writes we already model, and List#get takes it out. overrides: true so the rule reaches ArrayList and the other implementations rather than the interface alone.

`c6e707724` dropped `.java.lang.String#<serialized-value>#java.lang.Object` from all ten `ObjectMapper#readValue` overloads. Its reasoning -- the accessor hangs off arg(0), so it only type-checks when arg(0) may be a String -- is right, and it holds for nine of them. The tenth *is* the `java.lang.String` overload, where the base is a String and the read does type-check. Without it a tainted JSON string reaches `readValue` carrying its mark at `.java.lang.String#<serialized-value>#java.lang.Object` and the deserialized object comes back clean, because the surviving whole-object `arg(0) -> result` edge does not reach through that accessor (hertzbeat). Restores that one action only; the other nine stay removed. Verified: the deserialization shapes go from 13/18 to 18/18.

@approximate

…approximation The config passthrough `from: [arg(0), [*]] to: result` faithfully copies element sub-fields into the String result, but a field like File#path on a String is type-incompatible and the fact dies at the type checker. Replace with a @approximate dataflow model that expresses the actual semantics: iterate elements and call toString() on each. The engine analyzes the model body, so taint flows through the toString() call naturally. The toString() propagator in the sink rules keeps the taint alive through that call.

…ization overloads Commit 54749bcdd dropped <serialized-value> reads from all non-String readValue/readValues overloads, reasoning they cannot type-check against byte[]/Reader/InputStream. Restores all nine removed entries; the deserialization slot must not be deleted.

Restore concrete Spring slot types and the Go copy element model, and remove the incompatible whole-array-to-String copy.

Anchor surviving passthrough entries and copy actions to their order and formatting at 56a00bb so the cumulative diff contains only semantic additions, deletions, and changes.

misonijnik force-pushed the misonijnik/4-config branch 4 times, most recently from d196a75 to 00a8534 Compare July 30, 2026 08:57

misonijnik force-pushed the misonijnik/4-config branch 2 times, most recently from 308e9f1 to 8cf4e17 Compare August 12, 2026 09:24

misonijnik changed the base branch from main to misonijnik/3-rules August 12, 2026 09:46

misonijnik force-pushed the misonijnik/4-config branch from 8cf4e17 to 7c83fba Compare August 12, 2026 12:20

misonijnik force-pushed the misonijnik/4-config branch from 505719e to 37feeb3 Compare August 14, 2026 10:12

Saloed force-pushed the misonijnik/4-config branch from 37feeb3 to 9deaef4 Compare August 17, 2026 19:25

misonijnik marked this pull request as ready for review August 17, 2026 21:28

misonijnik force-pushed the misonijnik/4-config branch from 9deaef4 to 1d2f127 Compare August 18, 2026 15:13

misonijnik force-pushed the misonijnik/4-config branch from 1d2f127 to 2706f72 Compare August 18, 2026 15:45

Saloed force-pushed the misonijnik/4-config branch 2 times, most recently from 956ac26 to 48f5131 Compare August 19, 2026 22:20

misonijnik force-pushed the misonijnik/4-config branch 2 times, most recently from ad7f41e to 4504d6b Compare August 25, 2026 21:24

misonijnik force-pushed the misonijnik/4-config branch from 4504d6b to 23050b9 Compare August 25, 2026 21:25

misonijnik force-pushed the misonijnik/4-config branch 3 times, most recently from b3674a7 to ec591a8 Compare August 27, 2026 16:12

misonijnik added 29 commits September 24, 2026 00:36

fix(model): address passthrough review feedback

45389e9

Restore concrete Spring slot types and the Go copy element model, and remove the incompatible whole-array-to-String copy.

style(model): preserve config entry ordering

7a9b8f6

Anchor surviving passthrough entries and copy actions to their order and formatting at 56a00bb so the cumulative diff contains only semantic additions, deletions, and changes.

fix(model): preserve precise field-sensitive flows

97ffc1b

test(model): enforce semantic diff coverage

6415efd

fix(model): enforce precise field-sensitive passthroughs

b6dfd46

test(model): enforce stable config entry ordering

3d97527

test(model): remove config coverage tracking harness

5d92cd0

fix(model): restore precise passthrough flows

e55c5e1

revert(ci): leave PR scope validation unchanged

821459e

fix(ci): paginate changed PR files

efcfd99

fix(model): align field-sensitive passthrough copies

ca182e4

refactor(model): carry nio buffer fields together

8b9cc03

style(model): restore baseline config ordering

9a8fb97

Saloed force-pushed the misonijnik/4-config branch from 722fb85 to 9a8fb97 Compare September 23, 2026 21:36

Navigation Menu

feat(model): Make passthrough models more precise - #315

feat(model): Make passthrough models more precise#315
misonijnik wants to merge 80 commits into
misonijnik/3-rulesfrom
misonijnik/4-config

misonijnik commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

misonijnik commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants