Fix string format when escaped patterns are used (#6795) (#6805)

(cherry picked from commit e8da14e)

Co-authored-by: Manuel Álvarez Álvarez <manuel-alvarez-alvarez@users.noreply.github.com>

Author: smola

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java

@@ -5,6 +5,7 @@
 import static com.datadog.iast.taint.Ranges.mergeRanges;
 import static com.datadog.iast.taint.Tainteds.canBeTainted;
 import static com.datadog.iast.taint.Tainteds.getTainted;
+import static datadog.trace.api.telemetry.LogCollector.SEND_TELEMETRY;
 
 import com.datadog.iast.model.Range;
 import com.datadog.iast.taint.Ranges;
@@ -20,8 +21,12 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.Locale;
+import java.util.Map;
+import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
@@ -31,6 +36,10 @@ public class StringModuleImpl implements StringModule {
   private static final Pattern FORMAT_PATTERN =
       Pattern.compile("%(?<index>\\d+\\$)?([-#+ 0,(\\<]*)?(\\d+)?(\\.\\d+)?([tT])?([a-zA-Z%])");
 
+  /** Escaped format patterns * */
+  private static final Map<String, String> ESCAPED_PATTERNS =
+      Stream.of("%%", "%n").collect(Collectors.toMap(Function.identity(), String::format));
+
   private static final Ranged END = Ranged.build(Integer.MAX_VALUE, 0);
 
   private static final int NULL_STR_LENGTH = "null".length();
@@ -442,21 +451,32 @@ public void onStringFormat(
       final String placeholder = matcher.group();
       final Object parameter;
       final String formattedValue;
-      final String index = matcher.group("index");
-      if (index != null) {
-        // indexes are 1-based
-        final int parsedIndex = Integer.parseInt(index.substring(0, index.length() - 1)) - 1;
-        // remove the index before the formatting without increment the current state
-        parameter = parameters[parsedIndex];
-        formattedValue = String.format(locale, placeholder.replace(index, ""), parameter);
+      final TaintedObject taintedObject;
+      final String escaped = ESCAPED_PATTERNS.get(placeholder);
+      if (escaped != null) {
+        parameter = placeholder;
+        formattedValue = escaped;
+        taintedObject = null;
       } else {
-        parameter = parameters[paramIndex++];
-        formattedValue = String.format(locale, placeholder, parameter);
+        final String index = matcher.group("index");
+        if (index != null) {
+          // indexes are 1-based
+          final int parsedIndex = Integer.parseInt(index.substring(0, index.length() - 1)) - 1;
+          // remove the index before the formatting without increment the current state
+          parameter = parameters[parsedIndex];
+          formattedValue = String.format(locale, placeholder.replace(index, ""), parameter);
+        } else {
+          if (!checkParameterBounds(format, parameters, paramIndex)) {
+            return; // return without tainting the string in case of error
+          }
+          parameter = parameters[paramIndex++];
+          formattedValue = String.format(locale, placeholder, parameter);
+        }
+        taintedObject = to.get(parameter);
       }
       final Ranged placeholderPos = Ranged.build(matcher.start(), placeholder.length());
       final Range placeholderRange =
           addFormatTaintedRanges(placeholderPos, offset, formatRanges, finalRanges);
-      final TaintedObject taintedObject = to.get(parameter);
       final Range[] paramRanges = taintedObject == null ? null : taintedObject.getRanges();
       final int shift = placeholderPos.getStart() + offset;
       addParameterTaintedRanges(
@@ -473,6 +493,20 @@ public void onStringFormat(
     }
   }
 
+  private static boolean checkParameterBounds(
+      final String format, final Object[] parameters, int paramIndex) {
+    if (paramIndex < parameters.length) {
+      return true;
+    }
+    LOG.debug(
+        SEND_TELEMETRY,
+        "Error handling string format pattern {} with args {} at index {}",
+        format,
+        parameters.length,
+        paramIndex);
+    return false;
+  }
+
   @Override
   public void onStringFormat(
       @Nonnull final Iterable<String> literals,

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy

@@ -915,6 +915,10 @@ class StringModuleTest extends IastModuleImplTestBase {
     'Hello ==>%s<=='                | ['World!']                          | 'Hello ==>World!<==' // tainted placeholder [non tainted parameter]
     'He==>llo %s!<=='               | ['World']                           | 'He==>llo <====>World<====>!<==' // tainted placeholder (2) [non tainted parameter]
     'He==>llo %s!<=='               | ['W==>or<==ld']                     | 'He==>llo <==W==>or<==ld==>!<==' // tainted placeholder (3) [mixing with tainted parameter]
+    'Hello %n %n %s!%n'             | ['W==>or<==ld']                     | 'Hello \n \n W==>or<==ld!\n' // \n character
+    'Hello %% %% %s!%%'             | ['W==>or<==ld']                     | 'Hello % % W==>or<==ld!%' // % character
+    '==>Hello %n %s!<=='            | ['World']                           | '==>Hello <====>\n<====> <====>World<====>!<==' // \n character in tainted format (each placeholder generates a separate range)
+    '==>Hello %% %s!<=='            | ['World']                           | '==>Hello <====>%<====> <====>World<====>!<==' // % character in tainted format (each placeholder generates a separate range)
   }
 
   void 'onStringFormat literals: #literals args: #argsTainted'() {