diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/MastgTest.kt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/MastgTest.kt new file mode 100644 index 0000000000..a8e29022e9 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/MastgTest.kt @@ -0,0 +1,62 @@ +package org.owasp.mastestapp + +import android.content.Context +import android.util.Log +import java.io.File +import java.io.FileOutputStream +import java.io.IOException +import android.content.ContentValues +import android.os.Environment +import android.provider.MediaStore +import java.io.OutputStream + +class MastgTest (private val context: Context){ + + fun mastgTest(): String { + mastgTestApi() + mastgTestMediaStore() + return "SUCCESS!!\n\nFiles have been written with API and MediaStore" + } + + fun mastgTestApi() { + val externalStorageDir = context.getExternalFilesDir(null) + val fileName = File(externalStorageDir, "secret.txt") + val fileContent = "secr3tPa$$W0rd\n" + + try { + FileOutputStream(fileName).use { output -> + output.write(fileContent.toByteArray()) + Log.d("WriteExternalStorage", "File written to external storage successfully.") + } + } catch (e: IOException) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e) + } + } + + fun mastgTestMediaStore() { + try { + val resolver = context.contentResolver + var randomNum = (0..100).random().toString() + val contentValues = ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, "secretFile$randomNum.txt") + put(MediaStore.MediaColumns.MIME_TYPE, "text/plain") + put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS) + } + val textUri = resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues) + + textUri?.let { + val outputStream: OutputStream? = resolver.openOutputStream(it) + outputStream?.use { + it.write("MAS_API_KEY=8767086b9f6f976g-a8df76\n".toByteArray()) + it.flush() + } + Log.d("MediaStore", "File written to external storage successfully.") + } ?: run { + Log.e("MediaStore", "Error inserting URI to MediaStore.") + } + } catch (exception: Exception) { + Log.e("MediaStore", "Error writing file to URI from MediaStore", exception) + } + } +} + diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/demo.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/demo.md new file mode 100644 index 0000000000..495e0fd00f --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/demo.md @@ -0,0 +1,42 @@ +--- +platform: android +title: File System Snapshots from External Storage +tools: [adb] +code: [kotlin] +--- + +### Sample + +The snippet below shows sample code that creates two files in the external storage using the `getExternalFilesDir` method and the `MediaStore` API. + +{{ MastgTest.kt }} + +### Steps + +1. Install an app on your device. +2. Execute `run_before.sh`. +3. Open an app and exercise it to trigger file creations. +4. Execute `run_after.sh`. +5. Close the app once you finish testing. + +{{ run_before.sh # run_after.sh }} + +### Observation + +There is a list of all created files inside `output.txt`. + +{{ output.txt }} + +Their content is inside the `./new_files/` directory and contains: + +A password: + +{{ new_files/secret.txt }} + +And an API key: + +{{ new_files/secretFile75.txt }} + +### Evaluation + +This test fails because the files are not encrypted and contain sensitive data (a password and an API key). You can further confirm this by reverse engineering the app and inspecting the code. diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secret.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secret.txt new file mode 100644 index 0000000000..f5d3c92de6 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secret.txt @@ -0,0 +1 @@ +secr3tPa$$W0rd \ No newline at end of file diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secretFile75.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secretFile75.txt new file mode 100644 index 0000000000..5f17246f1a --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/new_files/secretFile75.txt @@ -0,0 +1 @@ +MAS_API_KEY=8767086b9f6f976g-a8df76 \ No newline at end of file diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/output.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/output.txt new file mode 100644 index 0000000000..7d684fc233 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/output.txt @@ -0,0 +1,2 @@ +/sdcard/Android/data/org.owasp.mastestapp/files/secret.txt +/sdcard/Download/secretFile75.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_after.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_after.sh new file mode 100755 index 0000000000..ecd2a03b0b --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_after.sh @@ -0,0 +1,10 @@ +#!/bin/bash + +# SUMMARY: List all files created after the creation date of a file created in run_before + +adb shell "find /sdcard/ -type f -newer /data/local/tmp/test_start" > output.txt +adb shell "rm /data/local/tmp/test_start" +mkdir -p new_files +while read -r line; do + adb pull "$line" ./new_files/ +done < output.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_before.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_before.sh new file mode 100755 index 0000000000..4187c12ac6 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/demo-1/run_before.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +# SUMMARY: This script creates a dummy file to mark a timestamp that we can use later +# on to identify files created during the app exercising + +adb shell "touch /data/local/tmp/test_start" diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/test.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/test.md new file mode 100644 index 0000000000..b9f55b9178 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-file-diff/test.md @@ -0,0 +1,28 @@ +--- +platform: android +title: Files Written to External Storage +type: [dynamic] +--- + +## Overview + +The goal of this test is to retrieve the files written to the external storage and inspect them regardless of the APIs used to write them. It uses a simple approach based on [file retrieval from the device storage](/MASTG/techniques/android/MASTG-TECH-0002) before and after the app is exercised to identify the files created during the app's execution and to check if they contain sensitive data. + +## Steps + +1. Make sure you have [adb](/MASTG/tools/android/MASTG-TOOL-0004) installed. +2. [Install the app](/MASTG/techniques/android/MASTG-TECH-0005). +3. Before running the app, [get the current list of files](/MASTG/techniques/android/MASTG-TECH-0002) in the external storage. +4. Exercise the app. +5. After running the app, retrieve the list of files in the external storage again. +6. Calculate the difference between the two lists. + +## Observation + +The output should contain a list of files that were created on the external storage during the app's execution. + +## Evaluation + +The test case fails if the files found above are not encrypted and leak sensitive data. + +To confirm this, you can [reverse engineer the app](/MASTG/techniques/android/MASTG-TECH-0017) and [inspect the code](/MASTG/techniques/android/MASTG-TECH-0023). diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/MastgTest.kt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/MastgTest.kt new file mode 100644 index 0000000000..ac2eb255ed --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/MastgTest.kt @@ -0,0 +1,61 @@ +package org.owasp.mastestapp + +import android.content.Context +import android.util.Log +import java.io.File +import java.io.FileOutputStream +import java.io.IOException +import android.content.ContentValues +import android.os.Environment +import android.provider.MediaStore +import java.io.OutputStream + +class MastgTest (private val context: Context){ + + fun mastgTest(): String { + mastgTestApi() + mastgTestMediaStore() + return "SUCCESS!!\n\nFiles have been written with API and MediaStore" + } + fun mastgTestApi() { + val externalStorageDir = context.getExternalFilesDir(null) + val fileName = File(externalStorageDir, "secret.txt") + val fileContent = "secr3tPa$$W0rd\n" + + try { + FileOutputStream(fileName).use { output -> + output.write(fileContent.toByteArray()) + Log.d("WriteExternalStorage", "File written to external storage successfully.") + } + } catch (e: IOException) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e) + } + } + + fun mastgTestMediaStore() { + try { + val resolver = context.contentResolver + var randomNum = (0..100).random().toString() + val contentValues = ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, "secretFile$randomNum.txt") + put(MediaStore.MediaColumns.MIME_TYPE, "text/plain") + put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS) + } + val textUri = resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues) + + textUri?.let { + val outputStream: OutputStream? = resolver.openOutputStream(it) + outputStream?.use { + it.write("MAS_API_KEY=8767086b9f6f976g-a8df76\n".toByteArray()) + it.flush() + } + Log.d("MediaStore", "File written to external storage successfully.") + } ?: run { + Log.e("MediaStore", "Error inserting URI to MediaStore.") + } + } catch (exception: Exception) { + Log.e("MediaStore", "Error writing file to URI from MediaStore", exception) + } + } +} + diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/demo.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/demo.md new file mode 100644 index 0000000000..f8ae6ccaf6 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/demo.md @@ -0,0 +1,43 @@ +--- +platform: android +title: External Storage APIs Tracing with Frida +tools: [frida] +code: [kotlin] +--- + +### Sample + +The snippet below shows sample code that creates two files in the external storage using the `getExternalFilesDir` method and the `MediaStore` API. + +{{ MastgTest.kt }} + +### Steps + +1. Ensure the app is running on the target device. +2. Execute `run.sh`. +3. Close the app once you finish testing. + +The `run.sh` script will inject a Frida script called `script.js`. + +{{ run.sh }} + +The Frida script will hook and log calls to `open` and `android.content.ContentResolver.insert`. + +{{ script.js }} + +### Observation + +In the output you can see the paths and the relevant stack trace which helps to identify the actual APIs used to write to external storage and their respective callers. + +{{ output.txt }} + +There are two files written to the external storage: + +- `/storage/emulated/0/Android/data/org.owasp.mastestapp/files/secret.txt` written using `java.io.FileOutputStream` from `org.owasp.mastestapp.MastgTest.mastgTestApi(MastgTest.kt:26)` +- `content://media/external/downloads/27` written using `android.content.ContentResolver.insert` from `org.owasp.mastestapp.MastgTest.mastgTestMediaStore(MastgTest.kt:44)` + +Note that the calls via `ContentResolver.insert` do not write directly to the file system, but to the `MediaStore` content provider, and therefore we can't see the actual file path, instead we see the `content://` URI. + +### Evaluation + +This test fails because the files are not encrypted and contain sensitive data (a password and an API key). You can further confirm this by reverse engineering the app and inspecting the code as well as retrieving the files from the device. diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/output.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/output.txt new file mode 100644 index 0000000000..7a7d577005 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/output.txt @@ -0,0 +1,145 @@ +[WARNING] Opening a file from external storage at: /storage/emulated/0/Android/data/org.owasp.mastestapp/files/secret.txt +Invoked from: java.lang.Exception + at libcore.io.Linux.open(Native Method) + at libcore.io.ForwardingOs.open(ForwardingOs.java:167) + at libcore.io.BlockGuardOs.open(BlockGuardOs.java:252) + at libcore.io.ForwardingOs.open(ForwardingOs.java:167) + at android.app.ActivityThread$AndroidOs.open(ActivityThread.java:7255) + at libcore.io.IoBridge.open(IoBridge.java:482) + at java.io.FileOutputStream.(FileOutputStream.java:235) + at java.io.FileOutputStream.(FileOutputStream.java:186) + at org.owasp.mastestapp.MastgTest.mastgTestApi(MastgTest.kt:26) + at org.owasp.mastestapp.MastgTest.mastgTest(MastgTest.kt:16) + at org.owasp.mastestapp.MainActivityKt$MyScreenContent$1$1$1.invoke(MainActivity.kt:73) + at org.owasp.mastestapp.MainActivityKt$MyScreenContent$1$1$1.invoke(MainActivity.kt:71) + at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke-k-4lQ0M(Clickable.kt:987) + at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke(Clickable.kt:981) + at androidx.compose.foundation.gestures.TapGestureDetectorKt$detectTapAndPress$2$1.invokeSuspend(TapGestureDetector.kt:255) + at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) + at kotlinx.coroutines.DispatchedTaskKt.resume(DispatchedTask.kt:177) + at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:166) + at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:474) + at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:508) + at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:497) + at kotlinx.coroutines.CancellableContinuationImpl.resumeWith(CancellableContinuationImpl.kt:368) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl$PointerEventHandlerCoroutine.offerPointerEvent(SuspendingPointerInputFilter.kt:665) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.dispatchPointerEvent(SuspendingPointerInputFilter.kt:544) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.onPointerEvent-H0pRuoY(SuspendingPointerInputFilter.kt:566) + at androidx.compose.foundation.AbstractClickablePointerInputNode.onPointerEvent-H0pRuoY(Clickable.kt:947) + at androidx.compose.foundation.AbstractClickableNode.onPointerEvent-H0pRuoY(Clickable.kt:795) + at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:317) + at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:303) + at androidx.compose.ui.input.pointer.NodeParent.dispatchMainEventPass(HitPathTracker.kt:185) + at androidx.compose.ui.input.pointer.HitPathTracker.dispatchChanges(HitPathTracker.kt:104) + at androidx.compose.ui.input.pointer.PointerInputEventProcessor.process-BIzXfog(PointerInputEventProcessor.kt:113) + at androidx.compose.ui.platform.AndroidComposeView.sendMotionEvent-8iAsVTc(AndroidComposeView.android.kt:1576) + at androidx.compose.ui.platform.AndroidComposeView.handleMotionEvent-8iAsVTc(AndroidComposeView.android.kt:1527) + at androidx.compose.ui.platform.AndroidComposeView.dispatchTouchEvent(AndroidComposeView.android.kt:1466) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:465) + at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1849) + at android.app.Activity.dispatchTouchEvent(Activity.java:3993) + at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:423) + at android.view.View.dispatchPointerEvent(View.java:13674) + at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:5482) + at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:5285) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) + at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) + at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4947) + at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) + at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:5004) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) + at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) + at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:7505) + at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:7474) + at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:7435) + at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:7630) + at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:188) + at android.os.MessageQueue.nativePollOnce(Native Method) + at android.os.MessageQueue.next(MessageQueue.java:336) + at android.os.Looper.loop(Looper.java:174) + at android.app.ActivityThread.main(ActivityThread.java:7356) + at java.lang.reflect.Method.invoke(Native Method) + at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) + at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930) + +[WARNING] Opening a file with MediaStore at: content://media/external/downloads/27 +Invoked from: java.lang.Exception + at android.content.ContentResolver.insert(Native Method) + at org.owasp.mastestapp.MastgTest.mastgTestMediaStore(MastgTest.kt:44) + at org.owasp.mastestapp.MastgTest.mastgTest(MastgTest.kt:17) + at org.owasp.mastestapp.MainActivityKt$MyScreenContent$1$1$1.invoke(MainActivity.kt:73) + at org.owasp.mastestapp.MainActivityKt$MyScreenContent$1$1$1.invoke(MainActivity.kt:71) + at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke-k-4lQ0M(Clickable.kt:987) + at androidx.compose.foundation.ClickablePointerInputNode$pointerInput$3.invoke(Clickable.kt:981) + at androidx.compose.foundation.gestures.TapGestureDetectorKt$detectTapAndPress$2$1.invokeSuspend(TapGestureDetector.kt:255) + at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) + at kotlinx.coroutines.DispatchedTaskKt.resume(DispatchedTask.kt:177) + at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:166) + at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:474) + at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:508) + at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:497) + at kotlinx.coroutines.CancellableContinuationImpl.resumeWith(CancellableContinuationImpl.kt:368) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl$PointerEventHandlerCoroutine.offerPointerEvent(SuspendingPointerInputFilter.kt:665) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.dispatchPointerEvent(SuspendingPointerInputFilter.kt:544) + at androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNodeImpl.onPointerEvent-H0pRuoY(SuspendingPointerInputFilter.kt:566) + at androidx.compose.foundation.AbstractClickablePointerInputNode.onPointerEvent-H0pRuoY(Clickable.kt:947) + at androidx.compose.foundation.AbstractClickableNode.onPointerEvent-H0pRuoY(Clickable.kt:795) + at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:317) + at androidx.compose.ui.input.pointer.Node.dispatchMainEventPass(HitPathTracker.kt:303) + at androidx.compose.ui.input.pointer.NodeParent.dispatchMainEventPass(HitPathTracker.kt:185) + at androidx.compose.ui.input.pointer.HitPathTracker.dispatchChanges(HitPathTracker.kt:104) + at androidx.compose.ui.input.pointer.PointerInputEventProcessor.process-BIzXfog(PointerInputEventProcessor.kt:113) + at androidx.compose.ui.platform.AndroidComposeView.sendMotionEvent-8iAsVTc(AndroidComposeView.android.kt:1576) + at androidx.compose.ui.platform.AndroidComposeView.handleMotionEvent-8iAsVTc(AndroidComposeView.android.kt:1527) + at androidx.compose.ui.platform.AndroidComposeView.dispatchTouchEvent(AndroidComposeView.android.kt:1466) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:3060) + at android.view.ViewGroup.dispatchTouchEvent(ViewGroup.java:2755) + at com.android.internal.policy.DecorView.superDispatchTouchEvent(DecorView.java:465) + at com.android.internal.policy.PhoneWindow.superDispatchTouchEvent(PhoneWindow.java:1849) + at android.app.Activity.dispatchTouchEvent(Activity.java:3993) + at com.android.internal.policy.DecorView.dispatchTouchEvent(DecorView.java:423) + at android.view.View.dispatchPointerEvent(View.java:13674) + at android.view.ViewRootImpl$ViewPostImeInputStage.processPointerEvent(ViewRootImpl.java:5482) + at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:5285) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) + at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) + at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4947) + at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) + at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:5004) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841) + at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807) + at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815) + at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788) + at android.view.ViewRootImpl.deliverInputEvent(ViewRootImpl.java:7505) + at android.view.ViewRootImpl.doProcessInputEvents(ViewRootImpl.java:7474) + at android.view.ViewRootImpl.enqueueInputEvent(ViewRootImpl.java:7435) + at android.view.ViewRootImpl$WindowInputEventReceiver.onInputEvent(ViewRootImpl.java:7630) + at android.view.InputEventReceiver.dispatchInputEvent(InputEventReceiver.java:188) + at android.os.MessageQueue.nativePollOnce(Native Method) + at android.os.MessageQueue.next(MessageQueue.java:336) + at android.os.Looper.loop(Looper.java:174) + at android.app.ActivityThread.main(ActivityThread.java:7356) + at java.lang.reflect.Method.invoke(Native Method) + at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492) + at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930) + diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/run.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/run.sh new file mode 100755 index 0000000000..aaa0e8bc32 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/run.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +# SUMMARY: This script uses frida to trace files that an app has opened since it spawned +# The script filters the output of frida-trace to print only the paths belonging to external +# storage but the the predefined list of external storage paths might not be complete. +# A sample output is shown in "output.txt". If the output is empty, it indicates that no external +# storage is used. + +frida \ + -U \ + -f org.owasp.mastestapp \ + -l script.js \ + -o output.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/script.js b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/script.js new file mode 100644 index 0000000000..e125fdd428 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/demo-1/script.js @@ -0,0 +1,27 @@ +// Intercept libc`open to make sure we cover all Java's APIs +Interceptor.attach(Module.getExportByName(null, 'open'), { + onEnter(args) { + const external_paths = ['/sdcard', '/storage/emulated'] + const path = Memory.readCString(ptr(args[0])); + external_paths.forEach(external_path => { + if(path.indexOf(external_path) == 0){ + console.log('[WARNING] Opening a file from external storage at:', path) + Java.performNow(function() { + console.log("Invoked from: "+Java.use("android.util.Log").getStackTraceString(Java.use("java.lang.Exception").$new())) + }); + } + }); + } +}); + +Java.perform(function() { + let ContentResolver = Java.use("android.content.ContentResolver"); + ContentResolver["insert"].implementation = function (uri, values) { + var result = this["insert"](uri, values); + console.log('[WARNING] Opening a file with MediaStore at:', result) + Java.performNow(function() { + console.log("Invoked from: "+Java.use("android.util.Log").getStackTraceString(Java.use("java.lang.Exception").$new())) + }); + return result + }; +}) diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/test.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/test.md new file mode 100644 index 0000000000..a04a23c9e3 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-dynamic-frida/test.md @@ -0,0 +1,30 @@ +--- +platform: android +title: Runtime Use of APIs to Access External Storage +apis: [Environment#getExternalStorageDirectory, Environment#getExternalStorageDirectory, Environment#getExternalFilesDir, Environment#getExternalCacheDir, FileOutputStream] +type: [dynamic] +--- + +## Overview + +Android apps use a variety of APIs to obtain a file path and store a file. Collecting a comprehensive list of these APIs can be challenging, especially if an app uses a third-party framework, loads code at runtime, or includes native code. The most effective approach to testing applications that write to device storage is usually dynamic analysis, and specifically [method tracing](/MASTG/techniques/android/MASTG-TECH-0033). + +## Steps + +1. Make sure you have [Frida](/MASTG/tools/generic/MASTG-TOOL-0031). installed. +2. Install the app. +3. Execute a script to spawn the app with Frida and log all interactions with files. +4. Navigate to the screen of the app that you want to analyse. +5. Close the app to stop Frida. + +The Frida script should log all file interactions by hooking into the relevant APIs such as `getExternalStorageDirectory`, `getExternalStoragePublicDirectory`, `getExternalFilesDir` or `FileOutPutStream`. You could also use `open` as a catch-all for file interactions. However, this won't catch all file interactions, such as those that use the `MediaStore` API and should be done with additional filtering as it can generate a lot of noise. + +## Observation + +The output should contain a list of files that the app wrote to the external storage during execution and, if possible, the APIs used to write them. + +## Evaluation + +The test case fails if the files found above are not encrypted and leak sensitive data. + +To confirm this, you can manually inspect the files using [adb shell](https://mas.owasp.org/MASTG/techniques/android/MASTG-TECH-0002/) to retrieve them from the device, and [reverse engineer the app](/MASTG/techniques/android/MASTG-TECH-0017) and [inspect the code](/MASTG/techniques/android/MASTG-TECH-0023). diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest.xml b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest.xml new file mode 100644 index 0000000000..9d1a99b7b9 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest_reversed.xml b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest_reversed.xml new file mode 100644 index 0000000000..dffe8e6411 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/AndroidManifest_reversed.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest.kt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest.kt new file mode 100644 index 0000000000..a210d84420 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest.kt @@ -0,0 +1,31 @@ +package org.owasp.mastestapp + +import android.content.Context +import android.os.Environment +import android.util.Log +import java.io.File +import java.io.FileOutputStream +import java.io.IOException + +class MastgTest (private val context: Context){ + + fun mastgTest(): String { + + val externalStorageDir = Environment.getExternalStorageDirectory() + + val fileName = File(externalStorageDir, "secret.txt") + val fileContent = "Secret not using scoped storage" + + try { + FileOutputStream(fileName).use { output -> + output.write(fileContent.toByteArray()) + Log.d("WriteExternalStorage", "File written to external storage successfully.") + } + } catch (e: IOException) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e) + return "ERROR!!\n\nError writing file to external storage. Do you have the MANAGE_EXTERNAL_STORAGE permission in the manifest and it's granted in 'All files access'?" + } + + return "SUCCESS!!\n\nFile $fileName with content $fileContent saved to $externalStorageDir" + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest_reversed.java b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest_reversed.java new file mode 100644 index 0000000000..20e9290d9a --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/MastgTest_reversed.java @@ -0,0 +1,46 @@ +package org.owasp.mastestapp; + +import android.content.Context; +import android.os.Environment; +import android.util.Log; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import kotlin.Metadata; +import kotlin.io.CloseableKt; +import kotlin.jvm.internal.Intrinsics; +import kotlin.text.Charsets; + +/* compiled from: MastgTest.kt */ +@Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0007"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "app_debug"}, k = 1, mv = {1, 9, 0}, xi = 48) +/* loaded from: classes4.dex */ +public final class MastgTest { + public static final int $stable = 8; + private final Context context; + + public MastgTest(Context context) { + Intrinsics.checkNotNullParameter(context, "context"); + this.context = context; + } + + public final String mastgTest() { + File externalStorageDir = Environment.getExternalStorageDirectory(); + File fileName = new File(externalStorageDir, "secret.txt"); + try { + FileOutputStream fileOutputStream = new FileOutputStream(fileName); + try { + FileOutputStream output = fileOutputStream; + byte[] bytes = "Secret not using scoped storage".getBytes(Charsets.UTF_8); + Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); + output.write(bytes); + Log.d("WriteExternalStorage", "File written to external storage successfully."); + CloseableKt.closeFinally(fileOutputStream, null); + return "File " + fileName + " with content Secret not using scoped storage saved to " + externalStorageDir; + } finally { + } + } catch (IOException e) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e); + return "Error writing file to external storage. Do you have the MANAGE_EXTERNAL_STORAGE permission in the manifest and it's granted?"; + } + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/demo.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/demo.md new file mode 100644 index 0000000000..e238674041 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/demo.md @@ -0,0 +1,36 @@ +--- +platform: android +title: App Writing to External Storage without Scoped Storage Restrictions +tools: [semgrep] +code: [kotlin, xml] +--- + +### Sample + +The snippet below shows sample code that creates a file in external storage without using scoped storage APIs. The `getExternalStorageDirectory` API returns a path to the root of the shared external storage (e.g. `/storage/emulated/0`). + +This requires special app access called ["All files access"](https://developer.android.com/preview/privacy/storage#all-files-access), so the `MANAGE_EXTERNAL_STORAGE` permission must be declared in the manifest file. + +{{ MastgTest.kt # MastgTest_reversed.java # AndroidManifest.xml # AndroidManifest_reversed.xml }} + +### Steps + +Let's run our semgrep rule against the reversed java code. + +{{ ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml }} + +And another one against the sample manifest file. + +{{ ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest.yml }} + +{{ run.sh }} + +### Observation + +The rule has identified one location in the code file where an API, `getExternalStorageDirectory`, is used to write to external storage as well as the location in the manifest file where the `MANAGE_EXTERNAL_STORAGE` permission is declared. + +{{ output.txt # output2.txt }} + +### Evaluation + +After reviewing the decompiled code at the location specified in the output (file and line number) we can conclude that the test fails because the file written by this instance contains sensitive data, specifically a password. diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output.txt new file mode 100644 index 0000000000..6ba66f2e44 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output.txt @@ -0,0 +1,11 @@ + + +┌────────────────┐ +│ 1 Code Finding │ +└────────────────┘ + + MastgTest_reversed.java + rules.mastg-android-data-unencrypted-shared-storage-no-user-interaction-external-api-public + [MASVS-STORAGE] Make sure to encrypt files at these locations if necessary + + 27┆ File externalStorageDir = Environment.getExternalStorageDirectory(); diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output2.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output2.txt new file mode 100644 index 0000000000..53d518b06c --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/output2.txt @@ -0,0 +1,11 @@ + + +┌────────────────┐ +│ 1 Code Finding │ +└────────────────┘ + + AndroidManifest_reversed.xml + rules.mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest + [MASVS-STORAGE] Make sure to encrypt files in external storage if necessary + + 2┆ diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/run.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/run.sh new file mode 100755 index 0000000000..195acb814a --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-1/run.sh @@ -0,0 +1,3 @@ +NO_COLOR=true semgrep -c ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml ./MastgTest_reversed.java --text -o output.txt + +NO_COLOR=true semgrep -c ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest.yml ./AndroidManifest_reversed.xml --text -o output2.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest.kt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest.kt new file mode 100644 index 0000000000..6c210660a3 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest.kt @@ -0,0 +1,30 @@ +package org.owasp.mastestapp + +import android.content.Context +import android.util.Log +import java.io.File +import java.io.FileOutputStream +import java.io.IOException + +class MastgTest (private val context: Context){ + + fun mastgTest(): String { + + val externalStorageDir = context.getExternalFilesDir(null) + + val fileName = File(externalStorageDir, "secret.txt") + val fileContent = "secr3tPa$$W0rd\n" + + try { + FileOutputStream(fileName).use { output -> + output.write(fileContent.toByteArray()) + Log.d("WriteExternalStorage", "File written to external storage successfully.") + } + } catch (e: IOException) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e) + return "ERROR!!\n\nError writing file to external storage" + } + + return "SUCCESS!!\n\nFile $fileName with content $fileContent saved to $externalStorageDir" + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest_reversed.java b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest_reversed.java new file mode 100644 index 0000000000..3c97d2a650 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/MastgTest_reversed.java @@ -0,0 +1,41 @@ +package org.owasp.mastestapp; + +import android.content.Context; +import android.util.Log; +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import kotlin.Metadata; +import kotlin.io.CloseableKt; +import kotlin.jvm.internal.Intrinsics; +import kotlin.text.Charsets; +/* compiled from: MastgTest.kt */ +@Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0007"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "app_debug"}, k = 1, mv = {1, 9, 0}, xi = 48) +/* loaded from: classes4.dex */ +public final class MastgTest { + public static final int $stable = 8; + private final Context context; + + public MastgTest(Context context) { + Intrinsics.checkNotNullParameter(context, "context"); + this.context = context; + } + + public final String mastgTest() { + File externalStorageDir = this.context.getExternalFilesDir(null); + File fileName = new File(externalStorageDir, "secret.txt"); + try { + FileOutputStream fileOutputStream = new FileOutputStream(fileName); + FileOutputStream output = fileOutputStream; + byte[] bytes = "secr3tPa$$W0rd\n".getBytes(Charsets.UTF_8); + Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); + output.write(bytes); + Log.d("WriteExternalStorage", "File written to external storage successfully."); + CloseableKt.closeFinally(fileOutputStream, null); + return "SUCCESS!!\n\nFile " + fileName + " with content secr3tPa$$W0rd\n saved to " + externalStorageDir; + } catch (IOException e) { + Log.e("WriteExternalStorage", "Error writing file to external storage", e); + return "ERROR!!\n\nError writing file to external storage"; + } + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/demo.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/demo.md new file mode 100644 index 0000000000..01221bd983 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/demo.md @@ -0,0 +1,30 @@ +--- +platform: android +title: App Writing to External Storage with Scoped Storage Restrictions +tools: [semgrep] +code: [kotlin] +--- + +### Sample + +The snippet below shows sample code that creates a file in external storage using the `getExternalFilesDir` API which returns a path to the app's external files directory (e.g. `/storage/emulated/0/Android/data/org.owasp.mastestapp/files`) and does not require any permissions to access. Scoped storage applies since the app targets Android 12 (API level 31) which is higher than Android 10 (API level 29). + +{{ MastgTest.kt # MastgTest_reversed.java }} + +### Steps + +Let's run our semgrep rule against the reversed java code. + +{{ ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml }} + +{{ run.sh }} + +### Observation + +The rule has identified one location in the code file where an API, `getExternalFilesDir`, is used to write to external storage with scoped storage restrictions. + +{{ output.txt }} + +### Evaluation + +After reviewing the decompiled code at the location specified in the output (file and line number) we can conclude that the test fails because the file written by this instance contains sensitive data, specifically a password. diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/output.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/output.txt new file mode 100644 index 0000000000..e0b6951351 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/output.txt @@ -0,0 +1,12 @@ + + +┌────────────────┐ +│ 1 Code Finding │ +└────────────────┘ + + MastgTest_reversed.java + rules.mastg-android-data-unencrypted-shared-storage-no-user-interaction-external-api-scoped + [MASVS-STORAGE] These locations might be accessible to other apps on Android 10 and below + given relevant permissions + + 25┆ File externalStorageDir = this.context.getExternalFilesDir(null); diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/run.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/run.sh new file mode 100755 index 0000000000..be9f7283bf --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-2/run.sh @@ -0,0 +1 @@ +NO_COLOR=true semgrep -c ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml ./MastgTest_reversed.java --text -o output.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest.kt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest.kt new file mode 100644 index 0000000000..c9a875fdb8 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest.kt @@ -0,0 +1,39 @@ +package org.owasp.mastestapp + +import android.content.ContentValues +import android.util.Log +import android.content.Context +import android.os.Environment +import android.provider.MediaStore +import java.io.OutputStream + +class MastgTest (private val context: Context){ + + fun mastgTest(): String { + try { + val resolver = context.contentResolver + val contentValues = ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, "secretFile.txt") + put(MediaStore.MediaColumns.MIME_TYPE, "text/plain") + put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS) + } + val textUri = resolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, contentValues) + + textUri?.let { + val outputStream: OutputStream? = resolver.openOutputStream(it) + outputStream?.use { + it.write("MAS_API_KEY=8767086b9f6f976g-a8df76\n".toByteArray()) + it.flush() + } + Log.d("MediaStore", "File written to external storage successfully.") + return "SUCCESS!!\n\nMediaStore inserted to $textUri" + } ?: run { + Log.e("MediaStore", "Error inserting URI to MediaStore.") + return "FAILURE!!\n\nMediaStore couldn't insert data." + } + } catch (exception: Exception) { + Log.e("MediaStore", "Error writing file to URI from MediaStore", exception) + return "FAILURE!!\n\nMediaStore couldn't insert data." + } + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest_reversed.java b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest_reversed.java new file mode 100644 index 0000000000..67789df039 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/MastgTest_reversed.java @@ -0,0 +1,59 @@ +package org.owasp.mastestapp; + +import android.content.ContentResolver; +import android.content.ContentValues; +import android.content.Context; +import android.net.Uri; +import android.os.Environment; +import android.provider.MediaStore; +import android.util.Log; +import java.io.OutputStream; +import kotlin.Metadata; +import kotlin.Unit; +import kotlin.io.CloseableKt; +import kotlin.jvm.internal.Intrinsics; +import kotlin.text.Charsets; +/* compiled from: MastgTest.kt */ +@Metadata(d1 = {"\u0000\u0018\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0002\n\u0002\u0010\u000e\n\u0000\b\u0007\u0018\u00002\u00020\u0001B\r\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0002\u0010\u0004J\u0006\u0010\u0005\u001a\u00020\u0006R\u000e\u0010\u0002\u001a\u00020\u0003X\u0082\u0004¢\u0006\u0002\n\u0000¨\u0006\u0007"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "(Landroid/content/Context;)V", "mastgTest", "", "app_debug"}, k = 1, mv = {1, 9, 0}, xi = 48) +/* loaded from: classes4.dex */ +public final class MastgTest { + public static final int $stable = 8; + private final Context context; + + public MastgTest(Context context) { + Intrinsics.checkNotNullParameter(context, "context"); + this.context = context; + } + + public final String mastgTest() { + try { + ContentResolver resolver = this.context.getContentResolver(); + ContentValues contentValues = new ContentValues(); + contentValues.put("_display_name", "secretFile.txt"); + contentValues.put("mime_type", "text/plain"); + contentValues.put("relative_path", Environment.DIRECTORY_DOCUMENTS); + Uri textUri = resolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues); + if (textUri != null) { + OutputStream outputStream = resolver.openOutputStream(textUri); + if (outputStream != null) { + OutputStream outputStream2 = outputStream; + OutputStream it = outputStream2; + byte[] bytes = "MAS_API_KEY=8767086b9f6f976g-a8df76\n".getBytes(Charsets.UTF_8); + Intrinsics.checkNotNullExpressionValue(bytes, "this as java.lang.String).getBytes(charset)"); + it.write(bytes); + it.flush(); + Unit unit = Unit.INSTANCE; + CloseableKt.closeFinally(outputStream2, null); + } + Log.d("MediaStore", "File written to external storage successfully."); + return "SUCCESS!!\n\nMediaStore inserted to " + textUri; + } + MastgTest mastgTest = this; + Log.e("MediaStore", "Error inserting URI to MediaStore."); + return "FAILURE!!\n\nMediaStore couldn't insert data."; + } catch (Exception exception) { + Log.e("MediaStore", "Error writing file to URI from MediaStore", exception); + return "FAILURE!!\n\nMediaStore couldn't insert data."; + } + } +} diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/demo.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/demo.md new file mode 100644 index 0000000000..a0847025f8 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/demo.md @@ -0,0 +1,32 @@ +--- +platform: android +title: App Writing to External Storage via the MediaStore API +tools: [semgrep] +code: [kotlin] +--- + +### Sample + +The snippet below shows sample code that uses the `MediaStore` API to write a file to shared storage in a path like `/storage/emulated/0/Download/` which does not require any permissions to access and is shared with other apps. + +{{ MastgTest.kt # MastgTest_reversed.java }} + +### Steps + +Let's run our semgrep rule against the sample code. + +{{ ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml }} + +{{ run.sh }} + +### Observation + +The rule has identified 2 locations that indicate a use of MediaStore API. + +{{ output.txt }} + +The first location is the import statement for the `MediaStore` API and the second location is where the `MediaStore` API is used to write to shared storage. + +### Evaluation + +After reviewing the decompiled code at the locations specified in the output (file and line number) we can conclude that the test fails because the file written by this instance contains sensitive data, specifically a API key. diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/output.txt b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/output.txt new file mode 100644 index 0000000000..22fa62a1b6 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/output.txt @@ -0,0 +1,13 @@ + + +┌─────────────────┐ +│ 2 Code Findings │ +└─────────────────┘ + + MastgTest_reversed.java + rules.mastg-android-data-unencrypted-shared-storage-no-user-interaction-mediastore + [MASVS-STORAGE] Make sure to want this data to be shared with other apps + + 8┆ import android.provider.MediaStore; + ⋮┆---------------------------------------- + 35┆ Uri textUri = resolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues); diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/run.sh b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/run.sh new file mode 100755 index 0000000000..be9f7283bf --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/demo-3/run.sh @@ -0,0 +1 @@ +NO_COLOR=true semgrep -c ../rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml ./MastgTest_reversed.java --text -o output.txt diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml new file mode 100644 index 0000000000..a736efaac4 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-apis.yml @@ -0,0 +1,36 @@ +rules: + - id: mastg-android-data-unencrypted-shared-storage-no-user-interaction-external-api-public + severity: WARNING + languages: + - java + metadata: + summary: This rule looks for methods that returns locations to "external storage" which is shared with other apps + message: "[MASVS-STORAGE] Make sure to encrypt files at these locations if necessary" + pattern-either: + - pattern: $X.getExternalStorageDirectory(...) + - pattern: $X.getExternalStoragePublicDirectory(...) + - pattern: $X.getDownloadCacheDirectory(...) + - pattern: Intent.ACTION_CREATE_DOCUMENT + - id: mastg-android-data-unencrypted-shared-storage-no-user-interaction-external-api-scoped + severity: WARNING + languages: + - java + metadata: + summary: This rule looks for methods that returns locations to "scoped external storage" + message: "[MASVS-STORAGE] These locations might be accessible to other apps on Android 10 and below given relevant permissions" + pattern-either: + - pattern: $X.getExternalFilesDir(...) + - pattern: $X.getExternalFilesDirs(...) + - pattern: $X.getExternalCacheDir(...) + - pattern: $X.getExternalCacheDirs(...) + - pattern: $X.getExternalMediaDirs(...) + - id: mastg-android-data-unencrypted-shared-storage-no-user-interaction-mediastore + severity: WARNING + languages: + - java + metadata: + summary: This rule scans for uses of MediaStore API that writes data to the external storage. This data can be accessed by other apps. + message: "[MASVS-STORAGE] Make sure to want this data to be shared with other apps" + pattern-either: + - pattern: import android.provider.MediaStore + - pattern: $X.MediaStore diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest.yml b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest.yml new file mode 100644 index 0000000000..93fa6dc9a5 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/rules/mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest.yml @@ -0,0 +1,15 @@ +rules: + - id: mastg-android-data-unencrypted-shared-storage-no-user-interaction-manifest + severity: WARNING + languages: + - generic + metadata: + summary: This rule scans for permissions that allows your app to write to external storage or shared storage + message: "[MASVS-STORAGE] Make sure to encrypt files in external storage if necessary" + pattern-either: + - pattern: WRITE_EXTERNAL_STORAGE + - pattern: MANAGE_EXTERNAL_STORAGE + - pattern: ACCESS_ALL_EXTERNAL_STORAGE + - pattern: requestLegacyExternalStorage="true" + - pattern: preserveLegacyExternalStorage="true" + - pattern: android:requestRawExternalStorageAccess="true" diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/test.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/test.md new file mode 100644 index 0000000000..36d36e19d2 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/android-data-unencrypted-shared-storage-no-user-interaction-static/test.md @@ -0,0 +1,37 @@ +--- +platform: android +title: References to APIs and Permissions for Accessing External Storage +apis: [Environment#getExternalStoragePublicDirectory, Environment#getExternalStorageDirectory, Environment#getExternalFilesDir, Environment#getExternalCacheDir, MediaStore, WRITE_EXTERNAL_STORAGE, MANAGE_EXTERNAL_STORAGE] +type: [static] +--- + +## Overview + +This test uses static analysis to look for uses of [APIs allowing an app to write to locations that are shared with other apps](../../../../../Document/0x05d-Testing-Data-Storage.md#external-storage) such as the external storage APIs or the `MediaStore` API as well as the relevant Android manifest storage-related permissions. + +This static test is great for identifying all code locations where the app is writing data to shared storage. However, it does not provide the actual data being written, and in some cases, the actual path in the device storage where the data is being written. Therefore, it is recommended to combine this test with others that take a dynamic approach, as this will provide a more complete view of the data being written to shared storage. + +## Steps + +1. [Reverse engineer the app](/MASTG/techniques/android/MASTG-TECH-0017). +2. Run a [static analysis](/MASTG/techniques/android/MASTG-TECH-0014) tool on the reverse engineered app targeting calls to any external storage APIs and Android manifest storage permissions. + +The static analysis tool should be able to identify all possible APIs and permissions used to write to shared storage, such as `getExternalStoragePublicDirectory`, `getExternalStorageDirectory`, `getExternalFilesDir`, `MediaStore`, `WRITE_EXTERNAL_STORAGE`, and `MANAGE_EXTERNAL_STORAGE`. See the [Android documentation](https://developer.android.com/training/data-storage/shared) for more information on these APIs and permissions. + +## Observation + +The output should contain a list of APIs and storage-related permissions used to write to shared storage and their code locations. + +## Evaluation + +The test case fails if: + +- the app has the proper permissions declared in the Android manifest (e.g. `WRITE_EXTERNAL_STORAGE`, `MANAGE_EXTERNAL_STORAGE`, etc.) +- **and** the data being written to shared storage is sensitive and not encrypted. + +To determine the latter, you may need to carefully [review the reversed code](/MASTG/techniques/android/MASTG-TECH-0023) and/or combine this test with others that take a dynamic approach, as this will provide a more complete view of the data being written to shared storage. + +## References + +- [Manage all files on a storage device](https://developer.android.com/training/data-storage/manage-all-files) +- [Access media files from shared storage](https://developer.android.com/training/data-storage/shared/media) diff --git a/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/weakness.md b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/weakness.md new file mode 100644 index 0000000000..d4a6aae1f7 --- /dev/null +++ b/weaknesses/MASVS-STORAGE/1-secure-data-storage/data-unencrypted-shared-storage-no-user-interaction/weakness.md @@ -0,0 +1,45 @@ +--- +title: Sensitive Data Stored Unencrypted in Shared Storage Requiring No User Interaction +alias: data-unencrypted-shared-storage-no-user-interaction +platform: [android] +profiles: [L1, L2] +mappings: + masvs-v1: [MSTG-STORAGE-2] + masvs-v2: [MASVS-STORAGE-1] + mastg-v1: [MASTG-TEST-0052, MASTG-TEST-0001] + cwe: [311] + android: https://developer.android.com/privacy-and-security/risks/sensitive-data-external-storage + +--- + +## Overview + +Apps frequently opt to store data in the external storage due to its larger capacity. However, this convenience comes with a potential security drawback. Once a malicious app is granted the relevant permissions, it can access this data without user consent or interaction at any time. Additionally, external storage like SD cards can be physically removed and read by a malicious actor. Even if the external storage is emulated by the system, the risk arises from improper file permissions or the misuse of APIs for saving files. In such cases, the files become vulnerable to unauthorized access, modifications and deletions, posing a security threat to the application. + +Developers may consider switching to Private Storage or Shared Storage Requiring User Interaction if they need more privacy and security. However, if the external storage is the most suitable for the app, it's a good practise to encrypt data stored in the external storage. Below you can find potential security impacts and mitigations linked to the use of the external storage. + +## Impact + +- **Loss of confidentiality**: An attacker can extract sensitive data stored externally, such as personal information and media like photos, documents, and audio files. + +- **Loss of secure material**: An attacker can extract passwords, cryptographic keys, and session tokens to facilitate additional attacks, such as identity theft or account takeover. + +- **Modification of app's behaviour**: An attacker can tamper with data used by the app, altering the app's logic. For example, they could modify a database describing the state of premium features or inject a malicious payload to enable further attacks such as SQL injection and Path Traversal. + +- **Modification of downloaded code**: An app can download new functionality from the Internet and store the executable code in external storage before loading it into the process. An attacker can modify this code before it is used by the app. + +## Modes of Introduction + +This threat is primarily a concern for Android devices since they permit the use of external storage. Even if a device lacks physical external storage, Android emulates it to provide access to the external storage API. + +- **Data Stored Unencrypted**: Sensitive data is stored in the external storage unencrypted. +- **Hardcoded Encryption Key**: Sensitive data is encrypted and stored in the external storage but the key is hardcoded inside the application. +- **Encryption Key Stored on Filesystem**: Sensitive data is encrypted and stored in the external storage but the key is stored alongside it or in another easily accessible location. +- **Encryption Used is Insufficient**: Sensitive data is encrypted but the encryption is not considered to be strong. +- **Reuse of encryption key**: The encryption key is shared between two devices owned by a single user, enabling the process of data cloning between these devices in the external storage. + +On iOS, apps cannot directly write to or read from the arbitrary locations, as compared to desktop operating system or Android. iOS maintains strict sandboxing rules, meaning apps can only access their own sandboxed file directories. + +## Mitigations + +Sensitive data stored in the external storage should be encrypted, and any keys used for data encryption should be protected by the device's hardware-backed keystore, where available. It is highly discouraged to include cryptographic keys hardcoded inside the application. You can also consider storing your files in the [private app sandbox or internal storage](https://developer.android.com/training/data-storage/app-specific#internal) and using [Android's EncryptedFile API wrapper for file encryption](https://developer.android.com/reference/androidx/security/crypto/EncryptedFile).