react-native-documents / document-picker

Document Picker for React Native
https://react-native-documents.github.io/
MIT License
1.33k stars 434 forks source link

fix(android): path traversal vulnerability #698

Closed vonovak closed 5 months ago

vonovak commented 5 months ago

Summary

This fixes a Path Traversal Vulnerability which was present on Android https://developer.android.com/privacy-and-security/risks/path-traversal.

According to CVSS spec, this is a high severity vulnerability see here.

The prerequisite is that the user has a malicious app installed on their phone that they can pick files from (think Google Drive or Dropbox, or some File browser app, which is malicious), and that the copyTo option is passed to the picking functions.

What could happen is that the fileName obtained from a Cursor when picking a file using the malicious' app DocumentProvider could contain special characters such as ../ which would change the destination that the file is being written to when using the copyTo option.

The vulnerability was reported by https://github.com/FixedOctocat

This can, generally speaking, lead to files being rewritten. In the context of React Native, this could lead to the js bundle of the application being swapped for another one, if user picked a malicious file from a malicious DocumentProvider, and the copyTo option is specified.

Test Plan

I tested the fix on a Android 10 device and Android 14 simulator. The fix for the issue follows the fix from the recommended mitigation.

What are the steps to reproduce (after prerequisites)?

Given a device or emulator, if you modify the first param passed to safeGetDestination to lead to a path outside of the cacheDir or FilesDir, copyFileToLocalStorage will not perform the copy the because safeGetDestination throws a IllegalArgumentException.

Compatibility

OS Implemented
iOS
Android

Checklist

josh-thompson13 commented 4 months ago

This issue is fixed in V8.2.2 right?

Please update https://github.com/advisories/GHSA-pmgm-h3cc-m4hj to reflect that 8.2.2 if fine.

vonovak commented 4 months ago

Yes, it is fixed in 8.2.2 as well as 9.1.1