Per review feedback on #21741: File.canRead/canWrite/canExecute,
exists/isDirectory/isFile/isHidden only inspect a path, so move them
under the path-injection[read] sub-kind. Update TaintedPath.expected
and the experimental CWE-073 expected to match.
The sink-model generator and the experimental java/file-path-injection
query now observe the new path-injection[read] sub-kind for the
FileInputStream and Files.copy source-argument models.
- CWE-073 FilePathInjection.expected: refresh the models table for the
renamed kind on FileInputStream(File); alerts unchanged.
- modelgenerator Sinks.java: update the inline sink annotation for
copyFileToDirectory(Path,Path,CopyOption[]) Argument[0] to the new
path-injection[read] sub-kind, mirroring the library change.
Introduce a new Models-as-Data sink sub-kind path-injection[read] for
models that only read from or inspect a path. The general
java/path-injection query and its PathInjectionSanitizer barrier
continue to consider both path-injection and path-injection[read]
sinks, so no alerts are lost. The java/zipslip query deliberately
selects only path-injection sinks, since read-only accesses such as
ClassLoader.getResource or FileInputStream are outside the archive
extraction threat model.
Addresses https://github.com/github/codeql/issues/21606 along the lines
proposed on the issue thread: prefer path-injection[read] over a
[create] sub-kind so that miscategorizing a sink causes a false
positive (easy to spot) rather than a false negative.
- shared/mad/codeql/mad/ModelValidation.qll: allow path-injection[...]
as a valid sink kind.
- java/ql/lib/ext/*.model.yml: relabel the models that PR #12916
migrated from the historical read-file kind (plus the newer
ClassLoader resource-lookup variants that share the same read-only
semantics).
- java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll and
PathSanitizer.qll: select both path-injection and
path-injection[read] sinks/barriers.
- java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll: keep only
path-injection, with a comment explaining why path-injection[read]
is excluded.
- java/ql/test/query-tests/security/CWE-022/semmle/tests/ZipTest.java:
add m7 regression covering the Dubbo-style classpath lookup from
issue #21606 and assert no alert is produced.
- Update TaintedPath.expected for the renamed kinds in the models list.
- Add change-notes under java/ql/lib/change-notes and
java/ql/src/change-notes.
`com.ctc.wstx.stax.WstxInputFactory` overrides `createXMLStreamReader`,
`createXMLEventReader` and `setProperty` from `XMLInputFactory`, so the
existing `XmlInputFactory` model in `XmlParsers.qll` does not match calls
where the static receiver type is `WstxInputFactory` (or its supertype
`org.codehaus.stax2.XMLInputFactory2`). Woodstox is vulnerable to XXE in
its default configuration, so these missed sinks were false negatives in
`java/xxe`.
This adds a scoped framework model under
`semmle/code/java/frameworks/woodstox/WoodstoxXml.qll` (registered in the
`Frameworks` module of `XmlParsers.qll`) that recognises these calls as
XXE sinks and treats the factory as safe when both
`javax.xml.stream.supportDTD` and
`javax.xml.stream.isSupportingExternalEntities` are disabled — mirroring
the existing `XMLInputFactory` safe-configuration logic.
Adds a dedicated test verifying that fields annotated with
@javax.validation.constraints.Pattern are recognized as sanitized
by RegexpCheckBarrier, in addition to the existing String.matches()
guard test.
secretQuestion is ambiguous: it could be the question text (not
sensitive) or a security question answer. Worse, the regex
secrets?(question) also matches secretQuestionAnswer, which is
clearly sensitive. Drop it to avoid false negatives.
The trust-boundary-violation query only recognized OWASP ESAPI validators
as sanitizers. ESAPI is rarely used in modern Java projects, while regex
validation via String.matches() and @javax.validation.constraints.Pattern
is the standard approach in Spring/Jakarta applications.
RegexpCheckBarrier already exists in Sanitizers.qll and is used by other
queries (e.g., RequestForgery). This wires it into TrustBoundaryConfig,
so patterns like input.matches("[a-zA-Z0-9]+") and @Pattern annotations
are recognized as sanitizers, consistent with the existing ESAPI treatment.
The sensitive-log query (CWE-532) lacked sanitizers for hashed or
encrypted data, while the sibling cleartext-storage query (CWE-312)
already recognized methods with "encrypt", "hash", or "digest" in their
names as sanitizers (CleartextStorageQuery.qll:86).
This adds an EncryptionBarrier to SensitiveLoggingQuery that applies the
same name-based heuristic, making the two queries consistent. Calls like
DigestUtils.sha256Hex(password) or hashPassword(secret) are no longer
flagged when their results are logged.
PathNormalizeSanitizer recognized Path.normalize() and
File.getCanonicalPath()/getCanonicalFile(), but not Path.toRealPath().
toRealPath() is strictly stronger than normalize() (resolves symlinks
and verifies file existence in addition to normalizing ".." components),
and is functionally equivalent to File.getCanonicalPath() for the NIO.2
API. CERT FIO16-J and OWASP both recommend it for path traversal defense.
This adds toRealPath to PathNormalizeSanitizer alongside normalize,
reducing false positives for code using idiomatic NIO.2 path handling.
- Clarify that arithmeticUsedInBoundsCheck applies to if-condition
comparisons, not all comparisons
- Update expected test line numbers to reflect added test calls
The java/tainted-arithmetic query now recognizes when an arithmetic
expression appears directly as an operand of a comparison (e.g.,
`if (off + len > array.length)`). Such expressions are bounds checks,
not vulnerable computations, and are excluded via the existing
overflowIrrelevant predicate.
Add test cases for bounds-checking patterns that should not be flagged.
- Model Signature.getInstance() as CryptoAlgoSpec sink (previously only
Signature constructor was modeled)
- Add HMAC-based algorithms (HMACSHA1/256/384/512, HmacSHA1/256/384/512)
and PBKDF2 to the secure algorithm whitelist
- Fix XDH/X25519/X448 tests to use KeyAgreement.getInstance() instead of
KeyPairGenerator.getInstance() to match their key agreement semantics
- Add test cases for SHA384withECDSA, HMACSHA*, and PBKDF2WithHmacSHA1
from user-reported false positives
- Update change note to document all additions
