Skip to content

Commit b27caf4

Browse files
committed
fix(appsec): gate FileChannel write callback on write-capable open options
FileChannel.open() with READ-only options was incorrectly triggering the fileWritten callback, which could cause false positives in the zipslip rule (dog-920-110) when a read-only channel open with a traversal path coincided with a multipart zip upload in the same request. Split beforeOpen into two overload-specific methods so the OpenOption arguments can be inspected at the call site, mirroring the existing pattern in beforeRandomAccessFileOpened. Also fix a latent bug in AdviceGeneratorImpl: .sorted() without a comparator on ArgumentSpecification (which does not implement Comparable) would ClassCastException when an advice method captures a strict subset of a target method's arguments. Fixed with Comparator.comparingInt.
1 parent ddbaef8 commit b27caf4

File tree

2 files changed

+88
-9
lines changed

2 files changed

+88
-9
lines changed

dd-java-agent/instrumentation/java/java-io-1.8/src/main/java/datadog/trace/instrumentation/java/lang/FileChannelCallSite.java

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
import datadog.trace.agent.tooling.csi.CallSite;
44
import datadog.trace.api.appsec.RaspCallSites;
5+
import java.nio.file.OpenOption;
56
import java.nio.file.Path;
7+
import java.nio.file.StandardOpenOption;
8+
import java.util.Set;
69
import javax.annotation.Nullable;
710

811
@CallSite(
@@ -12,14 +15,61 @@ public class FileChannelCallSite {
1215

1316
@CallSite.Before(
1417
"java.nio.channels.FileChannel java.nio.channels.FileChannel.open(java.nio.file.Path, java.nio.file.OpenOption[])")
18+
public static void beforeOpenArray(
19+
@CallSite.Argument(0) @Nullable final Path path,
20+
@CallSite.Argument(1) @Nullable final OpenOption[] options) {
21+
if (path != null) {
22+
String pathStr = path.toString();
23+
FileIORaspHelper.INSTANCE.beforeFileLoaded(pathStr);
24+
if (hasWriteOption(options)) {
25+
FileIORaspHelper.INSTANCE.beforeFileWritten(pathStr);
26+
}
27+
}
28+
}
29+
1530
@CallSite.Before(
1631
"java.nio.channels.FileChannel java.nio.channels.FileChannel.open(java.nio.file.Path, java.util.Set, java.nio.file.attribute.FileAttribute[])")
17-
public static void beforeOpen(@CallSite.Argument(0) @Nullable final Path path) {
32+
public static void beforeOpenSet(
33+
@CallSite.Argument(0) @Nullable final Path path,
34+
@CallSite.Argument(1) @Nullable final Set<? extends OpenOption> options) {
1835
if (path != null) {
19-
// Fire both read and write callbacks: the WAF determines whether to block
20-
// based on the full request context (e.g. zipslip requires body.filenames too).
21-
FileIORaspHelper.INSTANCE.beforeFileLoaded(path.toString());
22-
FileIORaspHelper.INSTANCE.beforeFileWritten(path.toString());
36+
String pathStr = path.toString();
37+
FileIORaspHelper.INSTANCE.beforeFileLoaded(pathStr);
38+
if (hasWriteOption(options)) {
39+
FileIORaspHelper.INSTANCE.beforeFileWritten(pathStr);
40+
}
41+
}
42+
}
43+
44+
private static boolean hasWriteOption(@Nullable final OpenOption[] options) {
45+
if (options == null) {
46+
return false;
2347
}
48+
for (OpenOption opt : options) {
49+
if (isWriteOption(opt)) {
50+
return true;
51+
}
52+
}
53+
return false;
54+
}
55+
56+
private static boolean hasWriteOption(@Nullable final Set<? extends OpenOption> options) {
57+
if (options == null) {
58+
return false;
59+
}
60+
for (OpenOption opt : options) {
61+
if (isWriteOption(opt)) {
62+
return true;
63+
}
64+
}
65+
return false;
66+
}
67+
68+
private static boolean isWriteOption(final OpenOption opt) {
69+
return opt == StandardOpenOption.WRITE
70+
|| opt == StandardOpenOption.APPEND
71+
|| opt == StandardOpenOption.CREATE
72+
|| opt == StandardOpenOption.CREATE_NEW
73+
|| opt == StandardOpenOption.TRUNCATE_EXISTING;
2474
}
2575
}

dd-java-agent/instrumentation/java/java-io-1.8/src/test/groovy/datadog/trace/instrumentation/java/io/FileChannelCallSiteTest.groovy

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import java.nio.file.StandardOpenOption
77

88
class FileChannelCallSiteTest extends BaseIoRaspCallSiteTest {
99

10-
void 'test RASP FileChannel.open read-only fires beforeFileLoaded'() {
10+
void 'test RASP FileChannel.open read-only fires only beforeFileLoaded'() {
1111
setup:
1212
final helper = Mock(FileIORaspHelper)
1313
FileIORaspHelper.INSTANCE = helper
@@ -18,10 +18,10 @@ class FileChannelCallSiteTest extends BaseIoRaspCallSiteTest {
1818

1919
then:
2020
1 * helper.beforeFileLoaded(path.toString())
21-
1 * helper.beforeFileWritten(path.toString())
21+
0 * helper.beforeFileWritten(_)
2222
}
2323

24-
void 'test RASP FileChannel.open write fires beforeFileWritten'() {
24+
void 'test RASP FileChannel.open write fires both beforeFileLoaded and beforeFileWritten'() {
2525
setup:
2626
final helper = Mock(FileIORaspHelper)
2727
FileIORaspHelper.INSTANCE = helper
@@ -35,7 +35,7 @@ class FileChannelCallSiteTest extends BaseIoRaspCallSiteTest {
3535
1 * helper.beforeFileWritten(path.toString())
3636
}
3737

38-
void 'test RASP FileChannel.open with Set of options'() {
38+
void 'test RASP FileChannel.open with Set READ-only fires only beforeFileLoaded'() {
3939
setup:
4040
final helper = Mock(FileIORaspHelper)
4141
FileIORaspHelper.INSTANCE = helper
@@ -45,8 +45,37 @@ class FileChannelCallSiteTest extends BaseIoRaspCallSiteTest {
4545
when:
4646
TestFileChannelSuite.openWithSet(path, options).close()
4747

48+
then:
49+
1 * helper.beforeFileLoaded(path.toString())
50+
0 * helper.beforeFileWritten(_)
51+
}
52+
53+
void 'test RASP FileChannel.open with Set WRITE fires both callbacks'() {
54+
setup:
55+
final helper = Mock(FileIORaspHelper)
56+
FileIORaspHelper.INSTANCE = helper
57+
final path = temporaryFolder.resolve('test_rasp_fc_set_write.txt')
58+
final options = EnumSet.of(StandardOpenOption.WRITE, StandardOpenOption.CREATE)
59+
60+
when:
61+
TestFileChannelSuite.openWithSet(path, options).close()
62+
4863
then:
4964
1 * helper.beforeFileLoaded(path.toString())
5065
1 * helper.beforeFileWritten(path.toString())
5166
}
67+
68+
void 'test RASP FileChannel.open with no options fires only beforeFileLoaded'() {
69+
setup:
70+
final helper = Mock(FileIORaspHelper)
71+
FileIORaspHelper.INSTANCE = helper
72+
final path = newFile('test_rasp_fc_default.txt').toPath()
73+
74+
when:
75+
TestFileChannelSuite.openWithOptions(path).close()
76+
77+
then:
78+
1 * helper.beforeFileLoaded(path.toString())
79+
0 * helper.beforeFileWritten(_)
80+
}
5281
}

0 commit comments

Comments
 (0)