-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Port MASTG-TEST-0004: App Exposing Sensitive Data to Embedded Libraries #3485
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
0cdcd45
536aacb
525a75e
ea8e1ed
31d6209
52d0619
99e15f8
a12122a
4618723
13118f7
ff3bf25
a45c42f
ccb8c1c
2037585
7e4ecf2
9078204
fab5dea
4351d41
625d6b3
dc4e504
2e7adb8
1d02dae
4dea6df
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| --- | ||
| platform: android | ||
| title: Uses of Firebase Analytics APIs on Potential PII with semgrep | ||
| id: MASTG-DEMO-00de1 | ||
| code: [java] | ||
| test: MASTG-TEST-02te1 | ||
| --- | ||
|
|
||
| ## Sample | ||
|
|
||
| This sample demonstrates an Android application sending data to Firebase Analytics. | ||
|
|
||
| {{ MainActivity.kt # MastgTest.kt # build.gradle.kts.libs }} | ||
|
|
||
| ## Steps | ||
|
|
||
| Let's run our @MASTG-TOOL-0110 rule against the reversed Java code. | ||
|
|
||
| {{ ../../../../rules/mastg-android-usage-of-firebase-analytics.yml }} | ||
|
|
||
| {{ run.sh }} | ||
|
|
||
| ## Observation | ||
|
|
||
| The rule detected one instance where sensitive data might be sent to Firebase Analytics. | ||
|
|
||
| {{ 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 app is using the Firebase Analytics SDK. | ||
|
|
||
| > Note: Since user input sent to Analytics is dynamic, we have no indication of whether the data being sent is actually sensitive. This evaluation is out of scope for this demo. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| package org.owasp.mastestapp | ||
|
|
||
| import android.os.Bundle | ||
| import androidx.activity.ComponentActivity | ||
| import androidx.activity.compose.setContent | ||
| import androidx.activity.enableEdgeToEdge | ||
| import androidx.compose.foundation.layout.Column | ||
| import androidx.compose.foundation.layout.Spacer | ||
| import androidx.compose.foundation.layout.fillMaxWidth | ||
| import androidx.compose.foundation.layout.height | ||
| import androidx.compose.foundation.layout.padding | ||
| import androidx.compose.material3.Text | ||
| import androidx.compose.material3.TextField | ||
| import androidx.compose.runtime.Composable | ||
| import androidx.compose.runtime.getValue | ||
| import androidx.compose.runtime.mutableStateOf | ||
| import androidx.compose.runtime.remember | ||
| import androidx.compose.runtime.setValue | ||
| import androidx.compose.ui.Modifier | ||
| import androidx.compose.ui.graphics.Color | ||
| import androidx.compose.ui.platform.LocalContext | ||
| import androidx.compose.ui.platform.testTag | ||
| import androidx.compose.ui.text.AnnotatedString | ||
| import androidx.compose.ui.text.buildAnnotatedString | ||
| import androidx.compose.ui.text.SpanStyle | ||
| import androidx.compose.ui.text.withStyle | ||
| import androidx.compose.ui.text.font.FontFamily | ||
| import androidx.compose.ui.tooling.preview.Preview | ||
| import androidx.compose.ui.unit.dp | ||
| import androidx.compose.ui.unit.sp | ||
| import kotlinx.serialization.json.Json | ||
| import kotlinx.serialization.json.JsonArray | ||
| import kotlinx.serialization.json.decodeFromJsonElement | ||
|
|
||
| const val MASTG_TEXT_TAG = "mastgTestText" | ||
|
|
||
| class MainActivity : ComponentActivity() { | ||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| enableEdgeToEdge() | ||
| setContent { | ||
| MainScreen() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fun UpdateDisplayString( | ||
| defaultMessage: String, | ||
| displayString: AnnotatedString, | ||
| result: String | ||
| ): AnnotatedString { | ||
| return buildAnnotatedString { | ||
| append(defaultMessage) | ||
| try { | ||
| val jsonArrayFromString = Json.parseToJsonElement(result) as JsonArray | ||
| val demoResults = jsonArrayFromString.map { Json.decodeFromJsonElement<DemoResult>(it) } | ||
|
|
||
| for (demoResult in demoResults) { | ||
| when (demoResult.status) { | ||
| Status.PASS -> { | ||
| withStyle(style = SpanStyle(color = Color.Green)) { | ||
| append("MASTG-DEMO-${demoResult.demoId} demonstrated a successful test:\n${demoResult.message}\n\n") | ||
| } | ||
| } | ||
| Status.FAIL -> { | ||
| withStyle(style = SpanStyle(color = Color(0xFFFF9800))) { | ||
| append("MASTG-DEMO-${demoResult.demoId} demonstrated a failed test:\n${demoResult.message}\n\n") | ||
| } | ||
| } | ||
| Status.ERROR -> { | ||
| withStyle(style = SpanStyle(color = Color.Red)) { | ||
| append("MASTG-DEMO-${demoResult.demoId} failed:\n${demoResult.message}\n\n") | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| // not a valid set of DemoResult, so print the result without any parsing | ||
| append(result) | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| @Preview | ||
| @Composable | ||
| fun MainScreen() { | ||
| val defaultMessage = "Click \"Start\" to send the data.\n\n" | ||
| var displayString by remember { mutableStateOf(buildAnnotatedString { append(defaultMessage) }) } | ||
| var input by remember { mutableStateOf("") } | ||
| val context = LocalContext.current | ||
| val mastgTestClass = MastgTest(context) | ||
| // By default run the test in a separate thread, this ensures that network tests such as those using SSLSocket work properly. | ||
| // However, some tests which interact with UI elements need to run on the main thread. | ||
| // You can set shouldRunInMainThread = true in MastgTest.kt for those tests. | ||
| val runInMainThread = MastgTest::class.members | ||
| .find { it.name == "shouldRunInMainThread" } | ||
| ?.call(mastgTestClass) as? Boolean ?: false | ||
|
|
||
| BaseScreen( | ||
| onStartClick = { | ||
| if (runInMainThread) { | ||
| val result = mastgTestClass.mastgTest(input) | ||
| displayString = UpdateDisplayString(defaultMessage, displayString, result) | ||
| } else { | ||
| Thread { | ||
| val result = mastgTestClass.mastgTest(input) | ||
| android.os.Handler(android.os.Looper.getMainLooper()).post { | ||
| displayString = UpdateDisplayString(defaultMessage, displayString, result) | ||
| } | ||
| }.start() | ||
| } | ||
| } | ||
| ) { | ||
| Column(modifier = Modifier.padding(16.dp)) { | ||
| TextField( | ||
| value = input, | ||
| onValueChange = { input = it }, | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| label = { Text("Enter something to sent to Firebase Analytics") }, | ||
| singleLine = true | ||
| ) | ||
| Spacer(modifier = Modifier.height(8.dp)) | ||
| Text( | ||
| modifier = Modifier | ||
| .testTag(MASTG_TEXT_TAG), | ||
| text = displayString, | ||
| color = Color.White, | ||
| fontSize = 16.sp, | ||
| fontFamily = FontFamily.Monospace | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you can just use the code from 00de3 for both demos. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package org.owasp.mastestapp | ||
|
|
||
| import android.content.Context | ||
| import com.google.firebase.analytics.FirebaseAnalytics | ||
| import com.google.firebase.analytics.logEvent | ||
|
|
||
| class MastgTest(context: Context) { | ||
|
|
||
| val analytics = FirebaseAnalytics.getInstance(context) | ||
|
|
||
| fun mastgTest(userInput: String): String { | ||
| analytics.logEvent("start_test") { | ||
| param("input", userInput) | ||
| } | ||
|
|
||
| return "'start_test' event was sent to Firebase Analytics with user input: $userInput" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package org.owasp.mastestapp; | ||
|
|
||
| import android.content.Context; | ||
| import com.google.firebase.analytics.FirebaseAnalytics; | ||
| import com.google.firebase.analytics.ParametersBuilder; | ||
| import kotlin.Metadata; | ||
| import kotlin.jvm.internal.Intrinsics; | ||
|
|
||
| /* compiled from: MastgTest.kt */ | ||
| @Metadata(d1 = {"\u0000\"\n\u0002\u0018\u0002\n\u0002\u0010\u0000\n\u0000\n\u0002\u0018\u0002\n\u0002\b\u0003\n\u0002\u0018\u0002\n\u0002\b\u0003\n\u0002\u0010\u000e\n\u0002\b\u0002\b\u0007\u0018\u00002\u00020\u0001B\u000f\u0012\u0006\u0010\u0002\u001a\u00020\u0003¢\u0006\u0004\b\u0004\u0010\u0005J\u000e\u0010\n\u001a\u00020\u000b2\u0006\u0010\f\u001a\u00020\u000bR\u0011\u0010\u0006\u001a\u00020\u0007¢\u0006\b\n\u0000\u001a\u0004\b\b\u0010\t¨\u0006\r"}, d2 = {"Lorg/owasp/mastestapp/MastgTest;", "", "context", "Landroid/content/Context;", "<init>", "(Landroid/content/Context;)V", "analytics", "Lcom/google/firebase/analytics/FirebaseAnalytics;", "getAnalytics", "()Lcom/google/firebase/analytics/FirebaseAnalytics;", "mastgTest", "", "userInput", "app_debug"}, k = 1, mv = {2, 0, 0}, xi = 48) | ||
| /* loaded from: classes3.dex */ | ||
| public final class MastgTest { | ||
| public static final int $stable = 8; | ||
| private final FirebaseAnalytics analytics; | ||
|
|
||
| public MastgTest(Context context) { | ||
| Intrinsics.checkNotNullParameter(context, "context"); | ||
| FirebaseAnalytics firebaseAnalytics = FirebaseAnalytics.getInstance(context); | ||
| Intrinsics.checkNotNullExpressionValue(firebaseAnalytics, "getInstance(...)"); | ||
| this.analytics = firebaseAnalytics; | ||
| } | ||
|
|
||
| public final FirebaseAnalytics getAnalytics() { | ||
| return this.analytics; | ||
| } | ||
|
|
||
| public final String mastgTest(String userInput) { | ||
| Intrinsics.checkNotNullParameter(userInput, "userInput"); | ||
| FirebaseAnalytics $this$logEvent$iv = this.analytics; | ||
| ParametersBuilder builder$iv = new ParametersBuilder(); | ||
| builder$iv.param("input", userInput); | ||
| $this$logEvent$iv.logEvent("start_test", builder$iv.getZza()); | ||
| return "'start_test' event was sent to Firebase Analytics with user input: '" + userInput + "'"; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| implementation("com.google.firebase:firebase-analytics:23.0.0") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
|
|
||
|
|
||
| ┌────────────────┐ | ||
| │ 1 Code Finding │ | ||
| └────────────────┘ | ||
|
|
||
| MastgTest_reversed.java | ||
| ❱ rules.mastg-android-usage-of-firebase-analytics | ||
| [MASVS-PRIVACY] Data is being sent to Firebase Analytics | ||
|
|
||
| 32┆ $this$logEvent$iv.logEvent("start_test", builder$iv.getZza()); | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| NO_COLOR=true semgrep -c ../../../../rules/mastg-android-usage-of-firebase-analytics.yml ./MastgTest_reversed.java > output.txt |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,36 @@ | ||||||||||||
| --- | ||||||||||||
| platform: android | ||||||||||||
| title: Determine if Sensitive Data are sent to Firebase Analytics with Frida | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
| id: MASTG-DEMO-00de3 | ||||||||||||
| code: [kotlin] | ||||||||||||
| test: MASTG-TEST-02te3 | ||||||||||||
| --- | ||||||||||||
|
|
||||||||||||
| ## Sample | ||||||||||||
|
|
||||||||||||
| This sample demonstrates an Android application that sends sensitive user information to Firebase Analytics using the `logEvent` method. The app collects the user's blood type and user ID, which are considered sensitive data (health information), and transmits them to Firebase Analytics. | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use the language from the Data Safety section. This is data sharing:
Suggested change
|
||||||||||||
|
|
||||||||||||
| > Note: We cannot perform this test with static analysis because the parameters sent to Firebase Analytics are constructed dynamically at runtime. | ||||||||||||
|
|
||||||||||||
| {{ MainActivity.kt # MastgTest.kt # build.gradle.kts.libs }} | ||||||||||||
|
|
||||||||||||
| ## Steps | ||||||||||||
|
|
||||||||||||
| 1. Install the app on a device (@MASTG-TECH-0005) | ||||||||||||
| 2. Make sure you have @MASTG-TOOL-0001 installed on your machine and the frida-server running on the device | ||||||||||||
| 3. Run `run.sh` to spawn the app with Frida | ||||||||||||
| 4. Select a blood type from the dropdown | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can surely avoid this step. |
||||||||||||
| 5. Click the **Start** button | ||||||||||||
| 6. Stop the script by pressing `Ctrl+C` and/or `q` to quit the Frida CLI | ||||||||||||
|
|
||||||||||||
| {{ hooks.js # run.sh }} | ||||||||||||
|
|
||||||||||||
| ## Observation | ||||||||||||
|
|
||||||||||||
| The output shows all instances of `logEvent` calls to Firebase Analytics SDK that were found at runtime, along with the parameters being sent. A backtrace is also provided to help identify the location in the code. | ||||||||||||
|
|
||||||||||||
| {{ output.json }} | ||||||||||||
|
|
||||||||||||
| ## Evaluation | ||||||||||||
|
|
||||||||||||
| This test **fails** because sensitive data (`blood_type` parameter) is being sent to Firebase Analytics via the `logEvent` method for a particular user (`user_id` parameter). | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjust to the changes in "Sample" to align with the Data Safety section. Also this can only fail if it's not declared in the Data Safety section and/or the privacy policy. We need a technique to get the Data Safety section and another to get the privacy policy. |
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also nice, but I'd also avoid modifying the UI if it's not strictly needed. The simpler the UI the better. See same suggestion for the other demo.