Skip to content

Commit 7e8f539

Browse files
antonisantonis
authored
Merge branch 'main' into antonis/cleanup-sentrysdkwrapper-deadcode
2 parents f34feb1 + ca37dc6 commit 7e8f539

7 files changed

Lines changed: 319 additions & 15 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
### Fixes
1717

1818
- Stop the Hermes sampling profiler on React instance teardown to prevent `pthread_kill` SIGABRT when the JS thread is torn down with profiling active ([#6035](https://github.com/getsentry/sentry-react-native/pull/6035))
19+
- Restrict the URI scope of `getDataFromUri` on iOS and Android ([#6045](https://github.com/getsentry/sentry-react-native/pull/6045))
1920
- Restrict the Metro source-context middleware to files within the project root ([#6044](https://github.com/getsentry/sentry-react-native/pull/6044))
2021
- Escape `name` and `version` values when injecting release constants into the web bundle ([#6044](https://github.com/getsentry/sentry-react-native/pull/6044))
2122
- Mask the Sentry auth token in the `sentry.gradle` upload-task lifecycle log ([#6057](https://github.com/getsentry/sentry-react-native/pull/6057))

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@
128128
"plist@npm:3.1.0/@xmldom/xmldom": "^0.8.13",
129129
"@appium/support@npm:7.0.6/yauzl": "^3.2.1",
130130
"appium-ios-remotexpc@npm:0.36.0/@xmldom/xmldom": "^0.9.10",
131-
"appium-ios-simulator@npm:8.0.12/@xmldom/xmldom": "^0.9.10"
131+
"appium-ios-simulator@npm:8.0.12/@xmldom/xmldom": "^0.9.10",
132+
"postcss": "^8.5.10"
132133
},
133134
"version": "0.0.0",
134135
"name": "sentry-react-native",

‎packages/core/android/build.gradle‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,7 @@ dependencies {
5757
implementation 'com.facebook.react:react-native:+'
5858
api 'io.sentry:sentry-android:8.40.0'
5959
debugImplementation 'io.sentry:sentry-spotlight:8.40.0'
60+
61+
testImplementation 'junit:junit:4.13.2'
62+
testImplementation 'org.mockito:mockito-core:5.12.0'
6063
}

‎packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java‎

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import java.util.ArrayList;
7575
import java.util.HashMap;
7676
import java.util.List;
77+
import java.util.Locale;
7778
import java.util.Map;
7879
import java.util.Properties;
7980
import java.util.concurrent.CountDownLatch;
@@ -1083,10 +1084,26 @@ public String fetchNativePackageName() {
10831084
}
10841085

10851086
public void getDataFromUri(String uri, Promise promise) {
1087+
final Uri parsedUri;
1088+
try {
1089+
parsedUri = Uri.parse(uri);
1090+
} catch (Exception e) {
1091+
String msg = "Invalid uri: " + uri;
1092+
logger.log(SentryLevel.ERROR, msg);
1093+
promise.reject(new Exception(msg));
1094+
return;
1095+
}
1096+
1097+
if (!isAllowedUri(parsedUri, getReactApplicationContext())) {
1098+
String msg = "Unsupported uri scheme or location: " + uri;
1099+
logger.log(SentryLevel.ERROR, msg);
1100+
promise.reject(new Exception(msg));
1101+
return;
1102+
}
1103+
10861104
try {
1087-
Uri contentUri = Uri.parse(uri);
10881105
try (InputStream is =
1089-
getReactApplicationContext().getContentResolver().openInputStream(contentUri)) {
1106+
getReactApplicationContext().getContentResolver().openInputStream(parsedUri)) {
10901107
if (is == null) {
10911108
String msg = "File not found for uri: " + uri;
10921109
logger.log(SentryLevel.ERROR, msg);
@@ -1115,6 +1132,77 @@ public void getDataFromUri(String uri, Promise promise) {
11151132
}
11161133
}
11171134

1135+
@VisibleForTesting
1136+
static boolean isAllowedUri(@NotNull Uri uri, @Nullable Context ctx) {
1137+
final String scheme = uri.getScheme();
1138+
if (scheme == null) {
1139+
return false;
1140+
}
1141+
final String lowerScheme = scheme.toLowerCase(Locale.ROOT);
1142+
if ("content".equals(lowerScheme)) {
1143+
return isAllowedContentAuthority(uri.getAuthority(), ctx);
1144+
}
1145+
return "file".equals(lowerScheme) && isPathUnderAppDirs(uri.getPath(), ctx);
1146+
}
1147+
1148+
// Allowlist is intentionally narrow:
1149+
// - `media` is what the in-SDK image picker flow returns.
1150+
// - the app's own FileProvider is needed when the host app exposes attachments via it.
1151+
// SAF document authorities
1152+
// (com.android.{externalstorage,providers.media,providers.downloads}.documents)
1153+
// are deliberately excluded: authority-only matching would let any caller read any document
1154+
// the app has been granted persistent SAF permission for, without verifying the URI was granted
1155+
// in the current attach flow.
1156+
@VisibleForTesting
1157+
static boolean isAllowedContentAuthority(@Nullable String authority, @Nullable Context ctx) {
1158+
if (authority == null) {
1159+
return false;
1160+
}
1161+
final String lower = authority.toLowerCase(Locale.ROOT);
1162+
if ("media".equals(lower)) {
1163+
return true;
1164+
}
1165+
if (ctx != null) {
1166+
final String pkg = ctx.getPackageName();
1167+
if (pkg != null && lower.equals(pkg.toLowerCase(Locale.ROOT) + ".fileprovider")) {
1168+
return true;
1169+
}
1170+
}
1171+
return false;
1172+
}
1173+
1174+
@VisibleForTesting
1175+
static boolean isPathUnderAppDirs(@Nullable String rawPath, @Nullable Context ctx) {
1176+
if (rawPath == null || rawPath.isEmpty()) {
1177+
return false;
1178+
}
1179+
if (ctx == null) {
1180+
return false;
1181+
}
1182+
try {
1183+
final String target = new File(rawPath).getCanonicalPath();
1184+
final File[] roots =
1185+
new File[] {
1186+
ctx.getFilesDir(),
1187+
ctx.getCacheDir(),
1188+
ctx.getExternalFilesDir(null),
1189+
ctx.getExternalCacheDir()
1190+
};
1191+
for (File root : roots) {
1192+
if (root == null) {
1193+
continue;
1194+
}
1195+
final String rootPath = root.getCanonicalPath();
1196+
if (target.equals(rootPath) || target.startsWith(rootPath + File.separator)) {
1197+
return true;
1198+
}
1199+
}
1200+
} catch (IOException e) {
1201+
return false;
1202+
}
1203+
return false;
1204+
}
1205+
11181206
public void encodeToBase64(ReadableArray array, Promise promise) {
11191207
byte[] bytes = new byte[array.size()];
11201208
for (int i = 0; i < array.size(); i++) {

‎packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
package io.sentry.react;
2+
3+
import static org.junit.Assert.assertFalse;
4+
import static org.junit.Assert.assertTrue;
5+
import static org.mockito.Mockito.mock;
6+
import static org.mockito.Mockito.when;
7+
8+
import android.content.Context;
9+
import android.net.Uri;
10+
import java.io.File;
11+
import java.io.IOException;
12+
import org.junit.Before;
13+
import org.junit.Rule;
14+
import org.junit.Test;
15+
import org.junit.rules.TemporaryFolder;
16+
17+
public class RNSentryUriValidationTest {
18+
19+
@Rule public TemporaryFolder tempFolder = new TemporaryFolder();
20+
21+
private Context ctx;
22+
private File filesDir;
23+
private File cacheDir;
24+
25+
@Before
26+
public void setUp() throws IOException {
27+
filesDir = tempFolder.newFolder("files");
28+
cacheDir = tempFolder.newFolder("cache");
29+
30+
ctx = mock(Context.class);
31+
when(ctx.getFilesDir()).thenReturn(filesDir);
32+
when(ctx.getCacheDir()).thenReturn(cacheDir);
33+
when(ctx.getExternalFilesDir(null)).thenReturn(null);
34+
when(ctx.getExternalCacheDir()).thenReturn(null);
35+
when(ctx.getPackageName()).thenReturn("com.example.app");
36+
}
37+
38+
private static Uri mockUri(String scheme, String authority, String path) {
39+
Uri u = mock(Uri.class);
40+
when(u.getScheme()).thenReturn(scheme);
41+
when(u.getAuthority()).thenReturn(authority);
42+
when(u.getPath()).thenReturn(path);
43+
return u;
44+
}
45+
46+
@Test
47+
public void rejectsNullScheme() {
48+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri(null, null, null), ctx));
49+
}
50+
51+
@Test
52+
public void rejectsUnknownScheme() {
53+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("http", "example.com", "/foo"), ctx));
54+
}
55+
56+
@Test
57+
public void rejectsAndroidResourceScheme() {
58+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("android.resource", "pkg", "/1"), ctx));
59+
}
60+
61+
@Test
62+
public void allowsContentMediaAuthority() {
63+
assertTrue(
64+
RNSentryModuleImpl.isAllowedUri(
65+
mockUri("content", "media", "/external/images/media/42"), ctx));
66+
}
67+
68+
@Test
69+
public void allowsContentSchemeCaseInsensitive() {
70+
assertTrue(RNSentryModuleImpl.isAllowedUri(mockUri("CONTENT", "Media", "/anything"), ctx));
71+
}
72+
73+
@Test
74+
public void rejectsSafDocumentsAuthorities() {
75+
// SAF authorities are excluded from the allowlist: authority-only matching cannot verify
76+
// that the URI was granted in the current attach flow versus a previously persisted grant.
77+
assertFalse(
78+
RNSentryModuleImpl.isAllowedUri(
79+
mockUri("content", "com.android.providers.media.documents", "/doc"), ctx));
80+
assertFalse(
81+
RNSentryModuleImpl.isAllowedUri(
82+
mockUri("content", "com.android.externalstorage.documents", "/doc"), ctx));
83+
assertFalse(
84+
RNSentryModuleImpl.isAllowedUri(
85+
mockUri("content", "com.android.providers.downloads.documents", "/doc"), ctx));
86+
}
87+
88+
@Test
89+
public void allowsAppOwnFileProviderAuthority() {
90+
assertTrue(
91+
RNSentryModuleImpl.isAllowedUri(
92+
mockUri("content", "com.example.app.fileprovider", "/shared"), ctx));
93+
}
94+
95+
@Test
96+
public void rejectsForeignContentAuthority() {
97+
assertFalse(
98+
RNSentryModuleImpl.isAllowedUri(
99+
mockUri("content", "com.attacker.evil.provider", "/evil"), ctx));
100+
}
101+
102+
@Test
103+
public void rejectsNullAuthorityOnContentScheme() {
104+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("content", null, "/x"), ctx));
105+
}
106+
107+
@Test
108+
public void rejectsFileSchemeWithNullContext() {
109+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, "/tmp/x"), null));
110+
}
111+
112+
@Test
113+
public void rejectsFileOutsideAppDirs() {
114+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, "/etc/passwd"), ctx));
115+
}
116+
117+
@Test
118+
public void rejectsFileTargetingInternalDatabases() {
119+
assertFalse(
120+
RNSentryModuleImpl.isAllowedUri(
121+
mockUri("file", null, "/data/data/com.example.app/databases/app.db"), ctx));
122+
}
123+
124+
@Test
125+
public void allowsFileInsideFilesDir() throws IOException {
126+
File target = new File(filesDir, "picked.jpg");
127+
target.createNewFile();
128+
assertTrue(
129+
RNSentryModuleImpl.isAllowedUri(mockUri("file", null, target.getAbsolutePath()), ctx));
130+
}
131+
132+
@Test
133+
public void allowsFileInsideCacheDir() throws IOException {
134+
File target = new File(cacheDir, "thumb.png");
135+
target.createNewFile();
136+
assertTrue(
137+
RNSentryModuleImpl.isAllowedUri(mockUri("file", null, target.getAbsolutePath()), ctx));
138+
}
139+
140+
@Test
141+
public void rejectsFileWithTraversalEscape() throws IOException {
142+
File outside = new File(tempFolder.getRoot(), "outside.txt");
143+
outside.createNewFile();
144+
String traversal = filesDir.getAbsolutePath() + "/../outside.txt";
145+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, traversal), ctx));
146+
}
147+
148+
@Test
149+
public void rejectsPrefixCollision() throws IOException {
150+
// If filesDir is <root>/files, a path <root>/filesEvil/victim must not match as inside
151+
// filesDir.
152+
File sibling = new File(tempFolder.getRoot(), "filesEvil");
153+
assertTrue(sibling.mkdirs());
154+
File victim = new File(sibling, "victim.txt");
155+
victim.createNewFile();
156+
assertFalse(
157+
RNSentryModuleImpl.isAllowedUri(mockUri("file", null, victim.getAbsolutePath()), ctx));
158+
}
159+
160+
@Test
161+
public void rejectsEmptyFilePath() {
162+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, ""), ctx));
163+
assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, null), ctx));
164+
}
165+
}

‎packages/core/ios/RNSentry.mm‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,45 @@ + (BOOL)captureReplayWithReturnValue
864864
#endif
865865
}
866866

867+
#if TARGET_OS_IPHONE || TARGET_OS_MACCATALYST
868+
static BOOL
869+
RNSentryIsPathUnderAllowedRoots(NSString *path)
870+
{
871+
if (path.length == 0) {
872+
return NO;
873+
}
874+
for (NSString *component in path.pathComponents) {
875+
if ([component isEqualToString:@".."]) {
876+
return NO;
877+
}
878+
}
879+
NSString *resolved = [path stringByResolvingSymlinksInPath];
880+
NSMutableArray<NSString *> *roots = [NSMutableArray arrayWithCapacity:3];
881+
NSString *tmp = NSTemporaryDirectory();
882+
if (tmp.length > 0) {
883+
[roots addObject:[tmp stringByResolvingSymlinksInPath]];
884+
}
885+
NSArray<NSString *> *caches
886+
= NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
887+
if (caches.firstObject) {
888+
[roots addObject:[caches.firstObject stringByResolvingSymlinksInPath]];
889+
}
890+
NSArray<NSString *> *docs
891+
= NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES);
892+
if (docs.firstObject) {
893+
[roots addObject:[docs.firstObject stringByResolvingSymlinksInPath]];
894+
}
895+
for (NSString *root in roots) {
896+
NSString *rootWithSlash =
897+
[root hasSuffix:@"/"] ? root : [root stringByAppendingString:@"/"];
898+
if ([resolved isEqualToString:root] || [resolved hasPrefix:rootWithSlash]) {
899+
return YES;
900+
}
901+
}
902+
return NO;
903+
}
904+
#endif
905+
867906
RCT_EXPORT_METHOD(getDataFromUri : (NSString *_Nonnull)uri resolve : (
868907
RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject)
869908
{
@@ -873,6 +912,13 @@ + (BOOL)captureReplayWithReturnValue
873912
reject(@"SentryReactNative", @"The provided URI is not a valid file:// URL", nil);
874913
return;
875914
}
915+
if (!RNSentryIsPathUnderAllowedRoots(fileURL.path)) {
916+
reject(@"SentryReactNative",
917+
@"The provided URI points outside the app's temporary, caches, or documents "
918+
@"directories",
919+
nil);
920+
return;
921+
}
876922
NSError *error = nil;
877923
NSData *fileData = [NSData dataWithContentsOfURL:fileURL options:0 error:&error];
878924
if (error || !fileData) {

0 commit comments

Comments
 (0)