diff --git a/CHANGELOG.md b/CHANGELOG.md index a78b933a47..d4bc6b5dc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ ### Fixes - 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)) +- Restrict `getDataFromUri` on iOS to the app's temporary, caches, and documents directories ([#6045](https://github.com/getsentry/sentry-react-native/pull/6045)) +- Restrict `getDataFromUri` on Android to known media and document content providers and app-internal file locations ([#6045](https://github.com/getsentry/sentry-react-native/pull/6045)) ### Dependencies diff --git a/packages/core/android/build.gradle b/packages/core/android/build.gradle index 8923cd0730..871096f0f4 100644 --- a/packages/core/android/build.gradle +++ b/packages/core/android/build.gradle @@ -57,4 +57,7 @@ dependencies { implementation 'com.facebook.react:react-native:+' api 'io.sentry:sentry-android:8.40.0' debugImplementation 'io.sentry:sentry-spotlight:8.40.0' + + testImplementation 'junit:junit:4.13.2' + testImplementation 'org.mockito:mockito-core:5.12.0' } diff --git a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java index 93eb2cea4c..3f3e7ebbe3 100644 --- a/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java +++ b/packages/core/android/src/main/java/io/sentry/react/RNSentryModuleImpl.java @@ -74,6 +74,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Properties; import java.util.concurrent.CountDownLatch; @@ -1083,10 +1084,26 @@ public String fetchNativePackageName() { } public void getDataFromUri(String uri, Promise promise) { + final Uri parsedUri; + try { + parsedUri = Uri.parse(uri); + } catch (Exception e) { + String msg = "Invalid uri: " + uri; + logger.log(SentryLevel.ERROR, msg); + promise.reject(new Exception(msg)); + return; + } + + if (!isAllowedUri(parsedUri, getReactApplicationContext())) { + String msg = "Unsupported uri scheme or location: " + uri; + logger.log(SentryLevel.ERROR, msg); + promise.reject(new Exception(msg)); + return; + } + try { - Uri contentUri = Uri.parse(uri); try (InputStream is = - getReactApplicationContext().getContentResolver().openInputStream(contentUri)) { + getReactApplicationContext().getContentResolver().openInputStream(parsedUri)) { if (is == null) { String msg = "File not found for uri: " + uri; logger.log(SentryLevel.ERROR, msg); @@ -1115,6 +1132,77 @@ public void getDataFromUri(String uri, Promise promise) { } } + @VisibleForTesting + static boolean isAllowedUri(@NotNull Uri uri, @Nullable Context ctx) { + final String scheme = uri.getScheme(); + if (scheme == null) { + return false; + } + final String lowerScheme = scheme.toLowerCase(Locale.ROOT); + if ("content".equals(lowerScheme)) { + return isAllowedContentAuthority(uri.getAuthority(), ctx); + } + if ("file".equals(lowerScheme)) { + return isPathUnderAppDirs(uri.getPath(), ctx); + } + return false; + } + + @VisibleForTesting + static boolean isAllowedContentAuthority(@Nullable String authority, @Nullable Context ctx) { + if (authority == null) { + return false; + } + final String lower = authority.toLowerCase(Locale.ROOT); + if ("media".equals(lower)) { + return true; + } + if ("com.android.providers.media.documents".equals(lower) + || "com.android.externalstorage.documents".equals(lower) + || "com.android.providers.downloads.documents".equals(lower)) { + return true; + } + if (ctx != null) { + final String pkg = ctx.getPackageName(); + if (pkg != null && lower.equals(pkg.toLowerCase(Locale.ROOT) + ".fileprovider")) { + return true; + } + } + return false; + } + + @VisibleForTesting + static boolean isPathUnderAppDirs(@Nullable String rawPath, @Nullable Context ctx) { + if (rawPath == null || rawPath.isEmpty()) { + return false; + } + if (ctx == null) { + return false; + } + try { + final String target = new File(rawPath).getCanonicalPath(); + final File[] roots = + new File[] { + ctx.getFilesDir(), + ctx.getCacheDir(), + ctx.getExternalFilesDir(null), + ctx.getExternalCacheDir() + }; + for (File root : roots) { + if (root == null) { + continue; + } + final String rootPath = root.getCanonicalPath(); + if (target.equals(rootPath) || target.startsWith(rootPath + File.separator)) { + return true; + } + } + } catch (IOException e) { + return false; + } + return false; + } + public void encodeToBase64(ReadableArray array, Promise promise) { byte[] bytes = new byte[array.size()]; for (int i = 0; i < array.size(); i++) { diff --git a/packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java b/packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java new file mode 100644 index 0000000000..5a3b09c345 --- /dev/null +++ b/packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java @@ -0,0 +1,163 @@ +package io.sentry.react; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import android.content.Context; +import android.net.Uri; +import java.io.File; +import java.io.IOException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class RNSentryUriValidationTest { + + @Rule public TemporaryFolder tempFolder = new TemporaryFolder(); + + private Context ctx; + private File filesDir; + private File cacheDir; + + @Before + public void setUp() throws IOException { + filesDir = tempFolder.newFolder("files"); + cacheDir = tempFolder.newFolder("cache"); + + ctx = mock(Context.class); + when(ctx.getFilesDir()).thenReturn(filesDir); + when(ctx.getCacheDir()).thenReturn(cacheDir); + when(ctx.getExternalFilesDir(null)).thenReturn(null); + when(ctx.getExternalCacheDir()).thenReturn(null); + when(ctx.getPackageName()).thenReturn("com.example.app"); + } + + private static Uri mockUri(String scheme, String authority, String path) { + Uri u = mock(Uri.class); + when(u.getScheme()).thenReturn(scheme); + when(u.getAuthority()).thenReturn(authority); + when(u.getPath()).thenReturn(path); + return u; + } + + @Test + public void rejectsNullScheme() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri(null, null, null), ctx)); + } + + @Test + public void rejectsUnknownScheme() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("http", "example.com", "/foo"), ctx)); + } + + @Test + public void rejectsAndroidResourceScheme() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("android.resource", "pkg", "/1"), ctx)); + } + + @Test + public void allowsContentMediaAuthority() { + assertTrue( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "media", "/external/images/media/42"), ctx)); + } + + @Test + public void allowsContentSchemeCaseInsensitive() { + assertTrue(RNSentryModuleImpl.isAllowedUri(mockUri("CONTENT", "Media", "/anything"), ctx)); + } + + @Test + public void allowsSafDocumentsAuthorities() { + assertTrue( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "com.android.providers.media.documents", "/doc"), ctx)); + assertTrue( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "com.android.externalstorage.documents", "/doc"), ctx)); + assertTrue( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "com.android.providers.downloads.documents", "/doc"), ctx)); + } + + @Test + public void allowsAppOwnFileProviderAuthority() { + assertTrue( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "com.example.app.fileprovider", "/shared"), ctx)); + } + + @Test + public void rejectsForeignContentAuthority() { + assertFalse( + RNSentryModuleImpl.isAllowedUri( + mockUri("content", "com.attacker.evil.provider", "/evil"), ctx)); + } + + @Test + public void rejectsNullAuthorityOnContentScheme() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("content", null, "/x"), ctx)); + } + + @Test + public void rejectsFileSchemeWithNullContext() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, "/tmp/x"), null)); + } + + @Test + public void rejectsFileOutsideAppDirs() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, "/etc/passwd"), ctx)); + } + + @Test + public void rejectsFileTargetingInternalDatabases() { + assertFalse( + RNSentryModuleImpl.isAllowedUri( + mockUri("file", null, "/data/data/com.example.app/databases/app.db"), ctx)); + } + + @Test + public void allowsFileInsideFilesDir() throws IOException { + File target = new File(filesDir, "picked.jpg"); + target.createNewFile(); + assertTrue( + RNSentryModuleImpl.isAllowedUri(mockUri("file", null, target.getAbsolutePath()), ctx)); + } + + @Test + public void allowsFileInsideCacheDir() throws IOException { + File target = new File(cacheDir, "thumb.png"); + target.createNewFile(); + assertTrue( + RNSentryModuleImpl.isAllowedUri(mockUri("file", null, target.getAbsolutePath()), ctx)); + } + + @Test + public void rejectsFileWithTraversalEscape() throws IOException { + File outside = new File(tempFolder.getRoot(), "outside.txt"); + outside.createNewFile(); + String traversal = filesDir.getAbsolutePath() + "/../outside.txt"; + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, traversal), ctx)); + } + + @Test + public void rejectsPrefixCollision() throws IOException { + // If filesDir is /files, a path /filesEvil/victim must not match as inside + // filesDir. + File sibling = new File(tempFolder.getRoot(), "filesEvil"); + assertTrue(sibling.mkdirs()); + File victim = new File(sibling, "victim.txt"); + victim.createNewFile(); + assertFalse( + RNSentryModuleImpl.isAllowedUri(mockUri("file", null, victim.getAbsolutePath()), ctx)); + } + + @Test + public void rejectsEmptyFilePath() { + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, ""), ctx)); + assertFalse(RNSentryModuleImpl.isAllowedUri(mockUri("file", null, null), ctx)); + } +} diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index 5376525bf1..4e14f87a98 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -864,6 +864,43 @@ + (BOOL)captureReplayWithReturnValue #endif } +#if TARGET_OS_IPHONE || TARGET_OS_MACCATALYST +static BOOL RNSentryIsPathUnderAllowedRoots(NSString *path) +{ + if (path.length == 0) { + return NO; + } + for (NSString *component in path.pathComponents) { + if ([component isEqualToString:@".."]) { + return NO; + } + } + NSString *resolved = [path stringByResolvingSymlinksInPath]; + NSMutableArray *roots = [NSMutableArray arrayWithCapacity:3]; + NSString *tmp = NSTemporaryDirectory(); + if (tmp.length > 0) { + [roots addObject:[tmp stringByResolvingSymlinksInPath]]; + } + NSArray *caches = NSSearchPathForDirectoriesInDomains( + NSCachesDirectory, NSUserDomainMask, YES); + if (caches.firstObject) { + [roots addObject:[caches.firstObject stringByResolvingSymlinksInPath]]; + } + NSArray *docs = NSSearchPathForDirectoriesInDomains( + NSDocumentDirectory, NSUserDomainMask, YES); + if (docs.firstObject) { + [roots addObject:[docs.firstObject stringByResolvingSymlinksInPath]]; + } + for (NSString *root in roots) { + NSString *rootWithSlash = [root hasSuffix:@"/"] ? root : [root stringByAppendingString:@"/"]; + if ([resolved isEqualToString:root] || [resolved hasPrefix:rootWithSlash]) { + return YES; + } + } + return NO; +} +#endif + RCT_EXPORT_METHOD(getDataFromUri : (NSString *_Nonnull)uri resolve : ( RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject) { @@ -873,6 +910,12 @@ + (BOOL)captureReplayWithReturnValue reject(@"SentryReactNative", @"The provided URI is not a valid file:// URL", nil); return; } + if (!RNSentryIsPathUnderAllowedRoots(fileURL.path)) { + reject(@"SentryReactNative", + @"The provided URI points outside the app's temporary, caches, or documents directories", + nil); + return; + } NSError *error = nil; NSData *fileData = [NSData dataWithContentsOfURL:fileURL options:0 error:&error]; if (error || !fileData) {