Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions packages/core/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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++) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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 <root>/files, a path <root>/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));
}
}
43 changes: 43 additions & 0 deletions packages/core/ios/RNSentry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,43 @@
#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<NSString *> *roots = [NSMutableArray arrayWithCapacity:3];
NSString *tmp = NSTemporaryDirectory();
if (tmp.length > 0) {
[roots addObject:[tmp stringByResolvingSymlinksInPath]];
}
NSArray<NSString *> *caches = NSSearchPathForDirectoriesInDomains(
NSCachesDirectory, NSUserDomainMask, YES);
if (caches.firstObject) {
[roots addObject:[caches.firstObject stringByResolvingSymlinksInPath]];
}
NSArray<NSString *> *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;
}

Check warning on line 901 in packages/core/ios/RNSentry.mm

View check run for this annotation

@sentry/warden / warden: code-review

No unit tests for iOS path validation function

The `RNSentryIsPathUnderAllowedRoots` function implements security-critical path validation logic but has no corresponding unit tests, unlike the Android implementation which has comprehensive tests in `RNSentryUriValidationTest.java`. This validation function handles path traversal prevention and symlink resolution - edge cases that should be tested to ensure bypass attempts are properly blocked.
Comment on lines +867 to +901
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No unit tests for iOS path validation function

The RNSentryIsPathUnderAllowedRoots function implements security-critical path validation logic but has no corresponding unit tests, unlike the Android implementation which has comprehensive tests in RNSentryUriValidationTest.java. This validation function handles path traversal prevention and symlink resolution - edge cases that should be tested to ensure bypass attempts are properly blocked.

Verification

Searched for iOS test files using patterns Test, Tests, Spec in packages/core/ios and found none. Searched for references to RNSentryIsPathUnderAllowedRoots and found only the implementation in RNSentry.mm. Confirmed Android has comprehensive tests in packages/core/android/src/test/java/io/sentry/react/RNSentryUriValidationTest.java covering traversal escape, prefix collision, and directory boundary cases.

Identified by Warden code-review · 6RE-5C3

#endif

RCT_EXPORT_METHOD(getDataFromUri : (NSString *_Nonnull)uri resolve : (
RCTPromiseResolveBlock)resolve rejecter : (RCTPromiseRejectBlock)reject)
{
Expand All @@ -873,6 +910,12 @@
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) {
Expand Down
Loading