Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: can't we merge both on a single topic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point 👍 Updated with 934ee43


### 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)) {
Comment thread
antonis marked this conversation as resolved.
Outdated
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,161 @@
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 @@ + (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<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;
}
Comment thread
sentry-warden[bot] marked this conversation as resolved.
#endif

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