Skip to content

Potential fix for code scanning alert no. 22: Use of externally-controlled format string#3041

Closed
cgivre wants to merge 1 commit intomasterfrom
alert-autofix-22
Closed

Potential fix for code scanning alert no. 22: Use of externally-controlled format string#3041
cgivre wants to merge 1 commit intomasterfrom
alert-autofix-22

Conversation

@cgivre
Copy link
Contributor

@cgivre cgivre commented Mar 13, 2026

Potential fix for https://github.com/apache/drill/security/code-scanning/22

At a high level, the fix is to prevent any user-controlled string from being used as a format string to String.format or similar APIs. Instead, the formatting helper should either (a) always treat the input as literal text, or (b) if formatting with arguments is desired, use a constant format string that incorporates the user data via %s, not as the pattern itself.

The single best fix here is to change UserException.Builder.message(String format, Object... args) so that it no longer treats the first parameter as a format string. We can preserve the public API (callers still pass message("x %s y", arg)), but internally we will build the message by concatenating the base text and the argument representations instead of re-invoking String.format. This completely eliminates the externally-controlled format-string sink while keeping behavior close enough for error reporting. Concretely, in common/src/main/java/org/apache/drill/common/exceptions/UserException.java, in the Builder.message method, we will replace:

if (args.length == 0) {
  message = format;
} else {
  message = String.format(format, args);
}

with logic that, when args.length > 0, converts format and args into a single string without using String.format on untrusted input, e.g., format + " " + Arrays.toString(args). This keeps existing functionality (message still contains both the base text and argument values) and removes the vulnerable call. No other files need code changes for this specific alert; the upstream flows into this method are then harmless, because the method no longer treats the parameter as a format string.

Implementation details:

  • Edit only within UserException.Builder.message in UserException.java.
  • Add an import for java.util.Arrays at the top of the file, as we will use Arrays.toString(args) to render the argument list.
  • Ensure no other behavior of UserException is changed (we preserve the wrapping semantics, context handling, etc.).
  • No modifications are required in BaseOptionManager, QueryResources, QueryWrapper, RestQueryRunner, or StatusResources for this specific CWE; once the sink is safe, those taint flows are no longer exploitable.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…olled format string

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@cgivre cgivre self-assigned this Mar 13, 2026
@cgivre cgivre added code-cleanup minor-update security backport-to-stable This bug fix is applicable to the latest stable release and should be considered for inclusion there labels Mar 13, 2026
@cgivre cgivre closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-stable This bug fix is applicable to the latest stable release and should be considered for inclusion there code-cleanup minor-update security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant