Skip to content

Commit 5d471a6

Browse files
committed
Fix charset parameter name matching to require exact boundary
Substring search could match 'xcharset=' as 'charset=', allowing a client-controlled decoy parameter to override the real charset. Now requires the match to be at position 0 or preceded by ';' or ' '.
1 parent bbaf10e commit 5d471a6

2 files changed

Lines changed: 41 additions & 20 deletions

File tree

internal-api/src/main/java/datadog/trace/api/http/MultipartContentDecoder.java

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,33 +20,40 @@ public static String decodeBytes(byte[] buf, int length, String contentType) {
2020

2121
public static Charset extractCharset(String contentType) {
2222
if (contentType == null) return null;
23-
int idx = indexOfIgnoreAsciiCase(contentType, "charset=");
24-
if (idx < 0) return null;
25-
int nameStart = idx + 8;
26-
int end = contentType.length();
27-
for (int i = nameStart; i < end; i++) {
28-
char c = contentType.charAt(i);
29-
if (c == ';' || c == ',' || c == ' ') {
30-
end = i;
31-
break;
23+
int searchFrom = 0;
24+
while (true) {
25+
int idx = indexOfIgnoreAsciiCase(contentType, "charset=", searchFrom);
26+
if (idx < 0) return null;
27+
// Require a parameter boundary before "charset=" so "xcharset=..." is not matched
28+
if (idx == 0 || contentType.charAt(idx - 1) == ';' || contentType.charAt(idx - 1) == ' ') {
29+
int nameStart = idx + 8;
30+
int end = contentType.length();
31+
for (int i = nameStart; i < end; i++) {
32+
char c = contentType.charAt(i);
33+
if (c == ';' || c == ',' || c == ' ') {
34+
end = i;
35+
break;
36+
}
37+
}
38+
String name = contentType.substring(nameStart, end).trim();
39+
if (name.length() > 1 && name.charAt(0) == '"' && name.charAt(name.length() - 1) == '"') {
40+
name = name.substring(1, name.length() - 1);
41+
}
42+
try {
43+
return Charset.forName(name);
44+
} catch (IllegalArgumentException e) {
45+
return null;
46+
}
3247
}
33-
}
34-
String name = contentType.substring(nameStart, end).trim();
35-
if (name.length() > 1 && name.charAt(0) == '"' && name.charAt(name.length() - 1) == '"') {
36-
name = name.substring(1, name.length() - 1);
37-
}
38-
try {
39-
return Charset.forName(name);
40-
} catch (IllegalArgumentException e) {
41-
return null;
48+
searchFrom = idx + 1;
4249
}
4350
}
4451

45-
private static int indexOfIgnoreAsciiCase(String s, String needle) {
52+
private static int indexOfIgnoreAsciiCase(String s, String needle, int fromIndex) {
4653
int sLen = s.length();
4754
int nLen = needle.length();
4855
outer:
49-
for (int i = 0, max = sLen - nLen; i <= max; i++) {
56+
for (int i = fromIndex, max = sLen - nLen; i <= max; i++) {
5057
for (int j = 0; j < nLen; j++) {
5158
if (Character.toLowerCase(s.charAt(i + j)) != needle.charAt(j)) {
5259
continue outer;

internal-api/src/test/java/datadog/trace/api/http/MultipartContentDecoderTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,20 @@ void extractCharsetFromStandardContentType(String contentType, String expectedCh
108108
assertEquals(expectedCharset, MultipartContentDecoder.extractCharset(contentType).name());
109109
}
110110

111+
@Test
112+
void extractCharsetIgnoresSubstringMatchInParameterName() {
113+
// "xcharset=UTF-16" must not match; the real "charset=UTF-8" that follows must be used
114+
assertEquals(
115+
"UTF-8",
116+
MultipartContentDecoder.extractCharset("text/plain; xcharset=UTF-16; charset=UTF-8")
117+
.name());
118+
}
119+
120+
@Test
121+
void extractCharsetReturnsNullWhenOnlySubstringMatchExists() {
122+
assertNull(MultipartContentDecoder.extractCharset("text/plain; xcharset=UTF-8"));
123+
}
124+
111125
@Test
112126
void extractCharsetHandlesAdditionalParameters() {
113127
assertEquals(

0 commit comments

Comments
 (0)