Skip to content

Commit fb89f9f

Browse files
committed
fix(appsec/jetty8): inspect all cached parts in GetPartAdvice, not just the singleton
When getPart("field") is the app's first multipart access, Jetty 8 parses the entire multipart stream and caches all parts in _multiPartInputStream, but only returns the one requested part. The previous advice forwarded just that singleton to AppSec, so any co-uploaded file parts were invisible to requestFilesFilenames — a WAF bypass if the app never called getParts() explicitly. Fix: read all cached parts via MultiPartInputStream.getParts() (reflected, cached handle) and fall back to the singleton only when reflection fails. Also remove the part==null early return: even if the requested field was not found, other file parts may have parsed. Add PartHelper.getAllParts(Object, Part) + FakeMpi unit tests.
1 parent 9f3d1d5 commit fb89f9f

3 files changed

Lines changed: 107 additions & 7 deletions

File tree

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.io.ByteArrayOutputStream;
1313
import java.io.IOException;
1414
import java.io.InputStream;
15+
import java.lang.reflect.Method;
1516
import java.util.ArrayList;
1617
import java.util.Collection;
1718
import java.util.Collections;
@@ -31,6 +32,37 @@ public class PartHelper {
3132

3233
private PartHelper() {}
3334

35+
// Cached reflection handle to MultiPartInputStream.getParts() — set once on first use.
36+
private static volatile Method mpiGetParts;
37+
38+
/**
39+
* Returns all parts from a {@code MultiPartInputStream} object (already-parsed, no re-trigger).
40+
* Falls back to a singleton of {@code singlePart} if reflection fails or the collection is empty.
41+
*/
42+
public static Collection<?> getAllParts(Object multiPartInputStream, Part singlePart) {
43+
if (multiPartInputStream != null) {
44+
Method m = mpiGetParts;
45+
if (m == null) {
46+
try {
47+
m = multiPartInputStream.getClass().getMethod("getParts");
48+
mpiGetParts = m;
49+
} catch (NoSuchMethodException ignored) {
50+
}
51+
}
52+
if (m != null) {
53+
try {
54+
@SuppressWarnings("unchecked")
55+
Collection<?> all = (Collection<?>) m.invoke(multiPartInputStream);
56+
if (all != null && !all.isEmpty()) {
57+
return all;
58+
}
59+
} catch (Exception ignored) {
60+
}
61+
}
62+
}
63+
return singlePart != null ? Collections.singletonList(singlePart) : Collections.emptyList();
64+
}
65+
3466
/**
3567
* Returns filenames found in {@code parts} by parsing each part's {@code Content-Disposition}
3668
* header for a {@code filename=} parameter.

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

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import java.io.IOException;
1616
import java.io.InputStream;
1717
import java.util.Collection;
18-
import java.util.Collections;
1918
import javax.servlet.http.Part;
2019
import net.bytebuddy.asm.Advice;
2120
import net.bytebuddy.implementation.bytecode.assign.Assigner;
@@ -154,10 +153,13 @@ static void after(
154153
}
155154

156155
/**
157-
* Fires AppSec events for a single-part upload via {@code getPart(String)}, which in Jetty 8.x
158-
* does NOT delegate to {@code getParts()} — it calls {@code
159-
* _multiPartInputStream.getPart(String)} directly. Without this advice, single-file uploads that
160-
* never call the public {@code getParts()} would be missed.
156+
* Fires AppSec events for requests whose first multipart access is {@code getPart(String)}.
157+
*
158+
* <p>In Jetty 8.x, {@code getPart(String)} parses and caches the entire multipart stream into
159+
* {@code _multiPartInputStream} but returns only the single requested part. If the app only calls
160+
* {@code getPart("field")} (a text field), any co-uploaded file parts would never reach {@code
161+
* requestFilesFilenames}. We therefore read all cached parts via {@code
162+
* MultiPartInputStream.getParts()} and fall back to the returned singleton only if that fails.
161163
*/
162164
@RequiresRequestContext(RequestContextSlot.APPSEC)
163165
public static class GetPartAdvice {
@@ -170,13 +172,18 @@ static boolean before() {
170172
static void after(
171173
@Advice.Enter boolean proceed,
172174
@Advice.Return Part part,
175+
@Advice.FieldValue(value = "_multiPartInputStream", typing = Assigner.Typing.DYNAMIC)
176+
Object multiPartInputStream,
173177
@ActiveRequestContext RequestContext reqCtx,
174178
@Advice.Thrown(readOnly = false) Throwable t) {
175179
CallDepthThreadLocalMap.decrementCallDepth(Part.class);
176-
if (!proceed || t != null || part == null) {
180+
if (!proceed || t != null) {
181+
return;
182+
}
183+
Collection<?> parts = PartHelper.getAllParts(multiPartInputStream, part);
184+
if (parts.isEmpty()) {
177185
return;
178186
}
179-
Collection<Part> parts = Collections.singletonList(part);
180187
t = PartHelper.fireBodyProcessedEvent(parts, reqCtx);
181188
if (t == null) {
182189
t = PartHelper.fireFilenamesEvent(parts, reqCtx);

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@ package datadog.trace.instrumentation.jetty8
33
import javax.servlet.http.Part
44
import spock.lang.Specification
55

6+
/** Minimal stand-in for {@code MultiPartInputStream}: exposes a {@code getParts()} method. */
7+
class FakeMpi {
8+
private final Collection parts
9+
10+
FakeMpi(Collection parts) {
11+
this.parts = parts
12+
}
13+
14+
Collection getParts() {
15+
parts
16+
}
17+
}
18+
619
class PartHelperTest extends Specification {
720

821
// ── extractFilenames ────────────────────────────────────────────────────────
@@ -191,6 +204,54 @@ class PartHelperTest extends Specification {
191204
PartHelper.extractFormFields(parts) == [a: ['x'], b: ['y']]
192205
}
193206

207+
// ── getAllParts ─────────────────────────────────────────────────────────────
208+
209+
def "getAllParts returns empty list when multiPartInputStream is null and singlePart is null"() {
210+
expect:
211+
PartHelper.getAllParts(null, null) == []
212+
}
213+
214+
def "getAllParts falls back to singleton when multiPartInputStream is null"() {
215+
given:
216+
def part = filePart('evil.php')
217+
218+
expect:
219+
PartHelper.getAllParts(null, part) == [part]
220+
}
221+
222+
def "getAllParts returns all parts from MultiPartInputStream.getParts()"() {
223+
given:
224+
def file = filePart('evil.php')
225+
def text = field('name', 'val')
226+
def mpi = new FakeMpi([file, text])
227+
228+
expect:
229+
PartHelper.getAllParts(mpi, null) == [file, text]
230+
}
231+
232+
def "getAllParts prefers full collection over singleton even when singlePart is provided"() {
233+
given:
234+
def file = filePart('evil.php')
235+
def other = field('name', 'val')
236+
def mpi = new FakeMpi([file, other])
237+
238+
expect:
239+
PartHelper.getAllParts(mpi, file) == [file, other]
240+
}
241+
242+
def "getAllParts falls back to singleton when getParts() throws"() {
243+
given:
244+
def part = filePart('fallback.jpg')
245+
def mpi = new Object() {
246+
Collection getParts() {
247+
throw new IOException("simulated failure")
248+
}
249+
}
250+
251+
expect:
252+
PartHelper.getAllParts(mpi, part) == [part]
253+
}
254+
194255
// ── helpers ─────────────────────────────────────────────────────────────────
195256

196257
/** Creates a stub Part that looks like a plain form field (no filename). */

0 commit comments

Comments
 (0)