Add java.util.Formatter related queries

Author: Marcono1234

codeql-custom-queries-java/queries/error-prone/format-string-from-string-concat.ql

@@ -0,0 +1,46 @@
+/**
+ * Finds calls to formatting methods such as `String.format` where the format string
+ * is created using String concatenation. This can be inefficient (for example for
+ * loggers where depending on the log level the formatted message is not needed),
+ * and can lead to exceptions if the format string created this way contains accidental
+ * format placeholders.
+ *
+ * For example:
+ * ```java
+ * // Error-prone: Can fail if `arg1` contains `%`
+ * String.format("first: " + arg1 + ", second: %s", arg2)
+ *
+ * // Should be rewritten to
+ * String.format("first: %s, second: %s", arg1, arg2)
+ * ```
+ *
+ * @kind problem
+ */
+
+import java
+import semmle.code.java.StringFormat
+import semmle.code.java.dataflow.DataFlow
+
+predicate isConstantString(Expr e) {
+  e.(CompileTimeConstantExpr).getType() instanceof TypeString
+  or
+  // Or number expression which is for example used as 'width' in the format string,
+  // e.g. `"%0" + width + "d"`
+  e.getType() instanceof NumericType
+  or
+  // Or expression as a whole is not a constant, but its operands are
+  isConstantString(e.(AddExpr).getLeftOperand()) and
+  isConstantString(e.(AddExpr).getRightOperand())
+  or
+  isConstantString(e.(ConditionalExpr).getTrueExpr()) and
+  isConstantString(e.(ConditionalExpr).getFalseExpr())
+}
+
+from FormattingCall formatCall, AddExpr formatStringExpr
+where
+  // Format string is created using String concat
+  DataFlow::localExprFlow(formatStringExpr, formatCall.getFormatArgument()) and
+  // Ignore if format string is constant and likely won't cause illegal format exceptions
+  not isConstantString(formatStringExpr)
+select formatStringExpr, "Format string created using String concatenation, used by $@", formatCall,
+  "this formatting call"

codeql-custom-queries-java/queries/error-prone/formatted-twice.ql

@@ -0,0 +1,55 @@
+/**
+ * Finds calls which format a string, for example using `String.format`, but where
+ * the format pattern string also comes from a formatted string. Such code is not only
+ * potentially redundant, but it can also lead to exceptions if the first format call
+ * accidentally adds `%`, which is interpreted as placeholder by the second format call.
+ *
+ * For example:
+ * ```java
+ * void checkArgument(boolean arg, String message, Object... messageArgs) {
+ *     if (!arg) {
+ *         throw new IllegalArgumentException(String.format(message, messageArgs));
+ *     }
+ * }
+ *
+ * ...
+ *
+ * // Bug: `checkArgument` will also call `String.format`
+ * // Fails for example if `value = "some%text"`
+ * checkArgument(isValid, String.format("invalid value: %s", value));
+ * ```
+ *
+ * @id todo
+ * @kind problem
+ */
+
+import java
+import semmle.code.java.StringFormat
+import semmle.code.java.dataflow.TaintTracking
+
+from
+  FormattingCall firstFormatCall, FormattingCall secondFormatCall, Expr secondFormatString,
+  string message, FormattingCall argExpr, string argMessage
+where
+  // Only consider `Formatter` formatting, don't consider mixed formatting variants,
+  // e.g. `logger.error(String.format(...))` since logger might not support rich formatting
+  not firstFormatCall.getSyntax().isLogger() and
+  not secondFormatCall.getSyntax().isLogger() and
+  secondFormatString = secondFormatCall.getFormatArgument() and
+  TaintTracking::localExprTaint(firstFormatCall, secondFormatString) and
+  // Ignore if first format call seems to use integer args to create the format layout for the second
+  // call, e.g. if `firstFormatCall` is `String.format("%%%ds", count)` (used by netty/netty in a test)
+  not forex(Expr arg | arg = firstFormatCall.getAnArgumentToBeFormatted() |
+    arg.getType().(NumericType).hasName(["int", "Integer"])
+  ) and
+  if secondFormatString = firstFormatCall
+  then (
+    message = "Formatting here is redundant because $@ performs formatting itself" and
+    argExpr = secondFormatCall and
+    argMessage = "the called method"
+  ) else (
+    message = "This format string argument has already been formatted $@ before" and
+    argExpr = firstFormatCall and
+    argMessage = "here"
+  )
+select secondFormatString, message, argExpr, argMessage

codeql-custom-queries-java/queries/error-prone/formatted-without-args.ql

@@ -0,0 +1,43 @@
+/**
+ * Finds formatting calls without arguments, such as `String.format("some message")`.
+ *
+ * This is not only redundant and can be simplified (for `PrintStream` and `PrintWriter`
+ * instead of `printf` `print` can be used), but can also cause an `IllegalFormatException`
+ * if the string happens to contain a malformed `%`. If the string can be influenced
+ * by an untrusted user, this could also be exploited for a denial-of-service attack.
+ *
+ * @kind problem
+ */
+
+import java
+
+predicate containsLineSeparator(Expr e) {
+  exists(e.(CompileTimeConstantExpr).getStringValue().indexOf("%n"))
+  or
+  // Or String concat where one operand contains "%n" (but the whole expression is not a constant)
+  e.getType() instanceof TypeString and containsLineSeparator(e.(AddExpr).getAnOperand())
+}
+
+from MethodAccess call, Method m
+where
+  m = call.getMethod() and
+  (
+    m.getDeclaringType() instanceof TypeString and
+    m.hasName(["format", "formatted"])
+    or
+    m.getDeclaringType()
+        .getASourceSupertype*()
+        .hasQualifiedName("java.io", ["PrintStream", "PrintWriter"]) and
+    m.hasName(["format", "printf"])
+    // Don't consider java.io.Console methods because there seems to be no direct alternative for writing
+    // a non-formatted string
+    // Don't consider java.util.Formatter because if only Formatter is available, using `formatter.out()` and
+    // writing to that is more cumbersome and possibly not as easy to understand
+  ) and
+  // And no varargs arguments have been provided
+  call.getNumArgument() < m.getNumberOfParameters() and
+  // And does not use `%n` in format string to get OS-dependent line separator
+  not containsLineSeparator(call.getAnArgument()) and
+  // Also cover `String.formatted` where the qualifier is the format string
+  not containsLineSeparator(call.getQualifier())
+select call, "Formatting call without arguments can be simplified"