Skip to content

Commit 0ce4bf1

Browse files
committed
fix(appsec/jetty): quote-aware Content-Disposition parser in PartHelper
Splitting the header on ';' naively truncated filenames that contain semicolons inside a quoted value, e.g. filename="shell;evil.php" would produce "shell" instead of the full name. Replace the split() loop with a quote-aware state-machine parser that skips semicolons inside quoted strings and handles backslash-escaped characters. Add test cases for semicolons in filenames, escaped quotes, and filename appearing before other parameters.
1 parent 1f2e2b3 commit 0ce4bf1

2 files changed

Lines changed: 66 additions & 7 deletions

File tree

  • dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/main/java/datadog/trace/instrumentation/jetty8/PartHelper.java

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,55 @@ public static Map<String, List<String>> extractFormFields(Collection<?> parts) {
7474
/**
7575
* Extracts the {@code filename} value from a {@code Content-Disposition} header, or {@code null}
7676
* if the part has no filename (i.e. it is a plain form field).
77+
*
78+
* <p>Uses a quote-aware parser so that semicolons inside a quoted filename (e.g. {@code
79+
* filename="shell;evil.php"}) are not mistaken for parameter separators.
7780
*/
7881
static String filenameFromPart(Part part) {
7982
String cd = part.getHeader("Content-Disposition");
8083
if (cd == null) {
8184
return null;
8285
}
83-
for (String token : cd.split(";")) {
84-
token = token.trim();
85-
if (token.startsWith("filename=")) {
86-
String name = token.substring("filename=".length()).trim();
87-
if (name.length() >= 2 && name.charAt(0) == '"' && name.charAt(name.length() - 1) == '"') {
88-
name = name.substring(1, name.length() - 1);
86+
int len = cd.length();
87+
int i = 0;
88+
while (i < len) {
89+
// Skip separators between parameters
90+
while (i < len && (cd.charAt(i) == ';' || cd.charAt(i) == ' ' || cd.charAt(i) == '\t')) {
91+
i++;
92+
}
93+
if (i >= len) break;
94+
// Read parameter name (up to '=' or ';')
95+
int nameStart = i;
96+
while (i < len && cd.charAt(i) != '=' && cd.charAt(i) != ';') {
97+
i++;
98+
}
99+
boolean isFilename = "filename".equalsIgnoreCase(cd.substring(nameStart, i).trim());
100+
if (i >= len || cd.charAt(i) == ';') {
101+
// Value-less token (e.g. "form-data") — skip
102+
continue;
103+
}
104+
i++; // skip '='
105+
String value;
106+
if (i < len && cd.charAt(i) == '"') {
107+
i++; // skip opening quote
108+
StringBuilder sb = new StringBuilder();
109+
while (i < len && cd.charAt(i) != '"') {
110+
if (cd.charAt(i) == '\\' && i + 1 < len) {
111+
i++; // consume escape backslash, add next char literally
112+
}
113+
sb.append(cd.charAt(i++));
114+
}
115+
if (i < len) i++; // skip closing quote
116+
value = sb.toString();
117+
} else {
118+
int valueStart = i;
119+
while (i < len && cd.charAt(i) != ';') {
120+
i++;
89121
}
90-
return name.isEmpty() ? null : name;
122+
value = cd.substring(valueStart, i).trim();
123+
}
124+
if (isFilename) {
125+
return value.isEmpty() ? null : value;
91126
}
92127
}
93128
return null;

dd-java-agent/instrumentation/jetty/jetty-appsec/jetty-appsec-8.1.3/src/test/groovy/datadog/trace/instrumentation/jetty8/PartHelperTest.groovy

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,30 @@ class PartHelperTest extends Specification {
107107
PartHelper.filenameFromPart(p) == null
108108
}
109109

110+
def "filenameFromPart preserves semicolons inside a quoted filename"() {
111+
given:
112+
Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename="shell;evil.php"' }
113+
114+
expect:
115+
PartHelper.filenameFromPart(p) == 'shell;evil.php'
116+
}
117+
118+
def "filenameFromPart handles escaped quote inside filename"() {
119+
given:
120+
Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; name="file"; filename="file\\"name.txt"' }
121+
122+
expect:
123+
PartHelper.filenameFromPart(p) == 'file"name.txt'
124+
}
125+
126+
def "filenameFromPart handles filename before other parameters"() {
127+
given:
128+
Part p = Stub(Part) { getHeader('Content-Disposition') >> 'form-data; filename="first.txt"; name="file"' }
129+
130+
expect:
131+
PartHelper.filenameFromPart(p) == 'first.txt'
132+
}
133+
110134
// ── extractFormFields ───────────────────────────────────────────────────────
111135

112136
def "extractFormFields returns empty map for null collection"() {

0 commit comments

Comments
 (0)