Skip to content

fix(java): detect String.formatted() in jdo-sqli - #4023

Open
Eljees wants to merge 2 commits into
semgrep:developfrom
Eljees:agent/jdo-sqli-formatted
Open

fix(java): detect String.formatted() in jdo-sqli#4023
Eljees wants to merge 2 commits into
semgrep:developfrom
Eljees:agent/jdo-sqli-formatted

Conversation

@Eljees

@Eljees Eljees commented Jul 28, 2026

Copy link
Copy Markdown

Fixes #3812

Problem

jdo-sqli matches String.format(...), but not the instance form added in Java 15:

// detected
pm.newQuery(UserEntity.class, new ArrayList(), String.format("id == %s", input));

// not detected before this change
pm.newQuery(UserEntity.class, new ArrayList(), "id == %s".formatted(input));

Both build the same query from the same input, so the newer spelling was a false negative.

Change

Adds a formatted() variant next to each existing String.format(...) pattern — the two pattern-inside forms for a query held in a local variable, and the direct $Q.$METHOD(...) / $PM.newQuery(...) calls.

The new patterns are written as "...".formatted($X,...), i.e. they require at least one argument. A constant string with nothing interpolated ("select * from Config".formatted()) cannot carry user input, so it stays unreported — that case is included in the test file as an ok annotation.

Tests

java/lang/security/audit/sqli/jdo-sqli.java gains a testJdoQueriesFormatted block covering both reproducers from the issue plus the argument-less negative case.

semgrep --test --config java/lang/security/audit/sqli/jdo-sqli.yaml java/lang/security/audit/sqli/jdo-sqli.java
1/1: ✓ All tests passed

The rule matched String.format(...) but not the instance form
"...%s...".formatted(input), so the same query built with the newer API
went unreported.

Add the formatted() variants next to each String.format() pattern. The
new patterns require at least one argument, so a constant string with no
interpolation is not flagged.

Signed-off-by: Eljees <3.14hell@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: da17133a16

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread java/lang/security/audit/sqli/jdo-sqli.yaml Outdated
Comment thread java/lang/security/audit/sqli/jdo-sqli.yaml Outdated
@Eljees

Eljees commented Jul 28, 2026

Copy link
Copy Markdown
Author

Both points are fair; here's how I'd weigh them.

Non-literal format templates (QUERY_TEMPLATE.formatted(input)): agreed that this is missed, but it's missed symmetrically today — the rule doesn't match String.format(QUERY_TEMPLATE, input) either. Widening the receiver to any expression would be a change in the rule's existing posture rather than a fix for #3812, so I'd rather not smuggle it into this PR. Happy to do it as a follow-up covering both String.format and formatted together, if you want that behaviour.

Format strings that consume no arguments ("select * from Config".formatted(input)): correct — Java ignores surplus arguments, so requiring $X doesn't prove the argument reaches the query. It narrows the obvious no-argument case but isn't airtight. Again this matches how the rule already treats String.format("select * from Config", input), so this PR leaves the rule no worse than it found it; a precise version would need to check that the template actually contains a conversion specifier, which the pattern language can't do here.

Tell me if you'd prefer the wider receiver, and I'll extend it consistently across both APIs.

Signed-off-by: Eljees <3.14hell@gmail.com>
@Eljees

Eljees commented Aug 10, 2026

Copy link
Copy Markdown
Author

Ping — open since 28 July, no review yet. All checks are green on 3981fcd.

jdo-sqli matches String.format(...) but not the Java 15 instance form "id == %s".formatted(input), so the newer spelling is a false negative. This adds a formatted() variant next to each existing String.format pattern, with test cases for both.

The two points from the automated review are answered in the thread: the format string that consumes no arguments is handled, and widening the receiver to non-literal templates is deliberately left out — String.format(QUERY_TEMPLATE, input) is missed symmetrically today, so that is a change in the rule's posture rather than a fix for #3812. Happy to fold it in if you would rather have both at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Java] false negative: jdo-sqli misses "...".formatted(input) pattern.

1 participant