Skip to content

Commit efec804

Browse files
l46kokcopybara-github
authored andcommitted
Prevent string.format injection when interpreter exception is being built
PiperOrigin-RevId: 600968606
1 parent 7d28e89 commit efec804

File tree

2 files changed

+23
-2
lines changed

2 files changed

+23
-2
lines changed

bundle/src/test/java/dev/cel/bundle/CelImplTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,15 @@ public void program_wrongTypeComprehensionThrows() throws Exception {
10451045
assertThat(e).hasMessageThat().contains("expected a list or a map");
10461046
}
10471047

1048+
@Test
1049+
public void program_stringFormatInjection_throwsEvaluationException() throws Exception {
1050+
Cel cel = standardCelBuilderWithMacros().build();
1051+
CelRuntime.Program program = cel.createProgram(cel.compile("{}['%2000222222s']").getAst());
1052+
1053+
CelEvaluationException e = assertThrows(CelEvaluationException.class, program::eval);
1054+
assertThat(e).hasMessageThat().contains("evaluation error");
1055+
}
1056+
10481057
@Test
10491058
public void program_emptyTypeProviderConfig() throws Exception {
10501059
Cel cel = standardCelBuilderWithMacros().build();

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.errorprone.annotations.CanIgnoreReturnValue;
1818
import com.google.errorprone.annotations.CheckReturnValue;
19+
import com.google.re2j.Pattern;
1920
import dev.cel.common.CelErrorCode;
2021
import dev.cel.common.CelRuntimeException;
2122
import dev.cel.common.annotations.Internal;
@@ -31,6 +32,8 @@
3132
*/
3233
@Internal
3334
public class InterpreterException extends Exception {
35+
// Allow format specifiers of %d, %f, %s and %n only.
36+
private static final Pattern ALLOWED_FORMAT_SPECIFIERS = Pattern.compile("%[^dfsn]");
3437
private final CelErrorCode errorCode;
3538

3639
public CelErrorCode getErrorCode() {
@@ -47,7 +50,7 @@ public static class Builder {
4750

4851
@SuppressWarnings({"AnnotateFormatMethod"}) // Format strings are optional.
4952
public Builder(String message, Object... args) {
50-
this.message = args.length > 0 ? String.format(message, args) : message;
53+
this.message = safeFormat(message, args);
5154
}
5255

5356
@SuppressWarnings({"AnnotateFormatMethod"}) // Format strings are optional.
@@ -64,7 +67,7 @@ public Builder(RuntimeException e, String message, Object... args) {
6467
this.cause = e;
6568
}
6669

67-
this.message = args.length > 0 ? String.format(message, args) : message;
70+
this.message = safeFormat(message, args);
6871
}
6972

7073
@CanIgnoreReturnValue
@@ -97,6 +100,15 @@ public InterpreterException build() {
97100
cause,
98101
errorCode);
99102
}
103+
104+
private static String safeFormat(String message, Object[] args) {
105+
if (args.length == 0) {
106+
return message;
107+
}
108+
109+
String sanitizedMessage = ALLOWED_FORMAT_SPECIFIERS.matcher(message).replaceAll("");
110+
return String.format(sanitizedMessage, args);
111+
}
100112
}
101113

102114
private InterpreterException(String message, Throwable cause, CelErrorCode errorCode) {

0 commit comments

Comments
 (0)