Add SafeStringFormatter. Address fuzzing issue around CelValidationEx…

…ception.

PiperOrigin-RevId: 601609078

common/internal/BUILD.bazel

@@ -67,3 +67,8 @@ java_library(
     name = "cel_descriptor_pools",
     exports = ["//common/src/main/java/dev/cel/common/internal:cel_descriptor_pools"],
 )
+
+java_library(
+    name = "safe_string_formatter",
+    exports = ["//common/src/main/java/dev/cel/common/internal:safe_string_formatter"],
+)

common/src/main/java/dev/cel/common/BUILD.bazel

@@ -64,6 +64,7 @@ java_library(
         ":common",
         "//:auto_value",
         "//common/annotations",
+        "//common/internal:safe_string_formatter",
         "//common/types:cel_types",
         "//common/types:type_providers",
         "@cel_spec//proto/cel/expr:expr_java_proto",

common/src/main/java/dev/cel/common/CelIssue.java

@@ -17,6 +17,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.errorprone.annotations.CheckReturnValue;
 import com.google.errorprone.annotations.Immutable;
+import dev.cel.common.internal.SafeStringFormatter;
 import java.util.Optional;
 import java.util.PrimitiveIterator;
 
@@ -76,7 +77,7 @@ public static CelIssue formatError(int line, int column, String message) {
   public String toDisplayString(CelSource source) {
     // Based onhttps://github.com/google/cel-go/blob/v0.5.1/common/error.go#L42.
     String result =
-        String.format(
+        SafeStringFormatter.format(
             "%s: %s:%d:%d: %s",
             getSeverity(),
             source.getDescription(),

common/src/main/java/dev/cel/common/CelValidationException.java

@@ -18,18 +18,21 @@
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
+import java.util.List;
 
 /** Base class for all checked exceptions explicitly thrown by the library during parsing. */
 public final class CelValidationException extends CelException {
 
   private static final Joiner JOINER = Joiner.on('\n');
+  // Truncates all errors beyond this limit in the message.
+  private static final int MAX_ERRORS_TO_REPORT = 1000;
 
   private final CelSource source;
   private final ImmutableList<CelIssue> errors;
 
   @VisibleForTesting
-  public CelValidationException(CelSource source, Iterable<CelIssue> errors) {
-    super(JOINER.join(Iterables.transform(errors, error -> error.toDisplayString(source))));
+  public CelValidationException(CelSource source, List<CelIssue> errors) {
+    super(safeJoinErrorMessage(source, errors));
     this.source = source;
     this.errors = ImmutableList.copyOf(errors);
   }
@@ -41,6 +44,21 @@ public CelValidationException(CelSource source, Iterable<CelIssue> errors) {
     this.errors = ImmutableList.copyOf(errors);
   }
 
+  private static String safeJoinErrorMessage(CelSource source, List<CelIssue> errors) {
+    if (errors.size() <= MAX_ERRORS_TO_REPORT) {
+      return JOINER.join(Iterables.transform(errors, error -> error.toDisplayString(source)));
+    }
+
+    List<CelIssue> truncatedErrors = errors.subList(0, MAX_ERRORS_TO_REPORT);
+    StringBuilder sb = new StringBuilder();
+    JOINER.appendTo(
+        sb, Iterables.transform(truncatedErrors, error -> error.toDisplayString(source)));
+    sb.append(
+        String.format("%n...and %d more errors (truncated)", errors.size() - MAX_ERRORS_TO_REPORT));
+
+    return sb.toString();
+  }
+
   /** Returns the {@link CelSource} that was being validated. */
   public CelSource getSource() {
     return source;

common/src/main/java/dev/cel/common/internal/BUILD.bazel

@@ -227,3 +227,14 @@ java_library(
         "@maven//:com_google_protobuf_protobuf_java",
     ],
 )
+
+java_library(
+    name = "safe_string_formatter",
+    srcs = ["SafeStringFormatter.java"],
+    tags = [
+    ],
+    deps = [
+        "//common/annotations",
+        "@maven//:com_google_re2j_re2j",
+    ],
+)

common/src/main/java/dev/cel/common/internal/SafeStringFormatter.java

@@ -0,0 +1,48 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dev.cel.common.internal;
+
+import com.google.re2j.Pattern;
+import dev.cel.common.annotations.Internal;
+
+/**
+ * {@link SafeStringFormatter} is a wrapper around JDK's {@link String#format}. It prevents any
+ * unsafe string.format calls by only allowing known formatting specifiers to be provided.
+ *
+ * <p>CEL Library Internals. Do Not Use.
+ */
+@Internal
+public final class SafeStringFormatter {
+  // Allow format specifiers of %d, %f, %s and %n only.
+  private static final Pattern FORBIDDEN_FORMAT_SPECIFIERS = Pattern.compile("%[^dfsn]");
+
+  /**
+   * Performs a safe {@link String#format}.
+   *
+   * @param format A format string. Only %d, %f, %s and %n are allowed as formatting specifiers. All
+   *     other formatting specifiers will be stripped out.
+   * @return A formatted string
+   */
+  public static String format(String format, Object... args) {
+    if (args.length == 0) {
+      return format;
+    }
+
+    String sanitizedMessage = FORBIDDEN_FORMAT_SPECIFIERS.matcher(format).replaceAll("");
+    return String.format(sanitizedMessage, args);
+  }
+
+  private SafeStringFormatter() {}
+}

common/src/test/java/dev/cel/common/BUILD.bazel

@@ -11,6 +11,7 @@ java_library(
     deps = [
         "//:java_truth",
         "//common",
+        "//common:compiler_common",
         "//common:features",
         "//common:options",
         "//common:proto_v1alpha1_ast",

common/src/test/java/dev/cel/common/CelValidationExceptionTest.java

@@ -0,0 +1,42 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dev.cel.common;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class CelValidationExceptionTest {
+
+  @Test
+  public void construct_withLargeErrorCount() {
+    ImmutableList.Builder<CelIssue> issueBuilder = ImmutableList.builder();
+    for (int i = 0; i < 1500; i++) {
+      issueBuilder.add(CelIssue.formatError(i + 1, i + 1, "generic error"));
+    }
+
+    CelValidationException celValidationException =
+        new CelValidationException(CelSource.newBuilder().build(), issueBuilder.build());
+
+    assertThat(celValidationException.getErrors()).hasSize(1500);
+    assertThat(celValidationException)
+        .hasMessageThat()
+        .endsWith("...and 500 more errors (truncated)");
+  }
+}

runtime/src/main/java/dev/cel/runtime/BUILD.bazel

@@ -54,6 +54,7 @@ java_library(
         "//common/internal:comparison_functions",
         "//common/internal:default_message_factory",
         "//common/internal:dynamic_proto",
+        "//common/internal:safe_string_formatter",
         "//common/types",
         "//common/types:type_providers",
         "@cel_spec//proto/cel/expr:expr_java_proto",

runtime/src/main/java/dev/cel/runtime/InterpreterException.java

@@ -16,10 +16,10 @@
 
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import com.google.errorprone.annotations.CheckReturnValue;
-import com.google.re2j.Pattern;
 import dev.cel.common.CelErrorCode;
 import dev.cel.common.CelRuntimeException;
 import dev.cel.common.annotations.Internal;
+import dev.cel.common.internal.SafeStringFormatter;
 import org.jspecify.nullness.Nullable;
 
 /**
@@ -32,8 +32,6 @@
  */
 @Internal
 public class InterpreterException extends Exception {
-  // Allow format specifiers of %d, %f, %s and %n only.
-  private static final Pattern ALLOWED_FORMAT_SPECIFIERS = Pattern.compile("%[^dfsn]");
   private final CelErrorCode errorCode;
 
   public CelErrorCode getErrorCode() {
@@ -50,7 +48,7 @@ public static class Builder {
 
     @SuppressWarnings({"AnnotateFormatMethod"}) // Format strings are optional.
     public Builder(String message, Object... args) {
-      this.message = safeFormat(message, args);
+      this.message = SafeStringFormatter.format(message, args);
     }
 
     @SuppressWarnings({"AnnotateFormatMethod"}) // Format strings are optional.
@@ -67,7 +65,7 @@ public Builder(RuntimeException e, String message, Object... args) {
         this.cause = e;
       }
 
-      this.message = safeFormat(message, args);
+      this.message = SafeStringFormatter.format(message, args);
     }
 
     @CanIgnoreReturnValue
@@ -100,15 +98,6 @@ public InterpreterException build() {
           cause,
           errorCode);
     }
-
-    private static String safeFormat(String message, Object[] args) {
-      if (args.length == 0) {
-        return message;
-      }
-
-      String sanitizedMessage = ALLOWED_FORMAT_SPECIFIERS.matcher(message).replaceAll("");
-      return String.format(sanitizedMessage, args);
-    }
   }
 
   private InterpreterException(String message, Throwable cause, CelErrorCode errorCode) {