From d98dccaf390d39b395f6246976fb9af2188d26cf Mon Sep 17 00:00:00 2001 From: Nickolas Dimitrakas Date: Thu, 17 Sep 2026 10:34:25 -0400 Subject: [PATCH 1/2] fix(lint): don't execute analyzed source when evaluating data-plan events DataplanDetector resolved MPEvent.Builder/CommerceEvent.Builder chains via unrestricted reflection (Class.forName/newInstance/invoke) driven by class and method names found in the analyzed project's own source, including anything reachable through a local variable's initializer chain. Restrict resolution to an explicit allowlist of the mParticle DTO/builder types this detector actually needs, and bound expression resolution depth so an unrelated failure mode can't escape the existing exception handling as a stack overflow. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 6 ++ .../java/com/mparticle/lints/Extensions.kt | 8 +- .../com/mparticle/lints/dtos/AllowedTypes.kt | 31 ++++++ .../com/mparticle/lints/dtos/Constructor.kt | 11 ++- .../com/mparticle/lints/dtos/MethodCall.kt | 10 +- .../mparticle/lints/dtos/ResolutionGuard.kt | 25 +++++ .../com/mparticle/lints/dtos/StaticFactory.kt | 9 +- .../java/com/mparticle/lints/Constants.kt | 13 ++- .../mparticle/lints/DataplanDetectorTest.kt | 96 +++++++++++++++++++ 9 files changed, 194 insertions(+), 15 deletions(-) create mode 100644 tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/AllowedTypes.kt create mode 100644 tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/ResolutionGuard.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index b23cfbd80..b6b041a7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Core + +#### Security + +- Restrict the Data Plan lint detector to only ever construct or invoke mParticle event/commerce builder types when evaluating analyzed source, instead of reflecting on any class named in a local variable's initializer chain ([#TBD](https://github.com/mParticle/mparticle-android-sdk/pull/TBD)) + ## [6.1.0] - 2026-09-15 ### Core diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/Extensions.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/Extensions.kt index 896f49756..6c52c2860 100644 --- a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/Extensions.kt +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/Extensions.kt @@ -9,6 +9,7 @@ import com.intellij.psi.PsiVariable import com.intellij.psi.impl.compiled.ClsMethodImpl import com.intellij.psi.impl.source.PsiClassReferenceType import com.intellij.psi.impl.source.PsiImmediateClassType +import com.mparticle.lints.dtos.AllowedTypes import com.mparticle.lints.dtos.Constructor import com.mparticle.lints.dtos.Expression import com.mparticle.lints.dtos.MethodCall @@ -238,7 +239,7 @@ internal fun UExpression.resolveChainedCalls(returnValue: Boolean, instance: Exp } } -internal fun Pair<*, *>.resolveToEnum(): Enum<*> { +internal fun Pair<*, *>.resolveToEnum(): Enum<*>? { val className = when (first) { is ClassId -> "${(first as ClassId).packageFqName}.${ @@ -247,6 +248,9 @@ internal fun Pair<*, *>.resolveToEnum(): Enum<*> { is String -> first as String else -> null } + if (!AllowedTypes.isAllowed(className)) { + return null + } return className?.let { className -> val enumName = second.toString() val constructor = @@ -255,7 +259,7 @@ internal fun Pair<*, *>.resolveToEnum(): Enum<*> { .methods .first { it.name == "valueOf" } constructor.invoke(null, enumName) - } as Enum<*> + } as? Enum<*> } internal fun List.resolve(): List = map { it.resolve() } diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/AllowedTypes.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/AllowedTypes.kt new file mode 100644 index 000000000..b5fb03cb0 --- /dev/null +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/AllowedTypes.kt @@ -0,0 +1,31 @@ +package com.mparticle.lints.dtos + +/** + * The Data Plan detector needs to compute the message body of an event builder chain found in + * analyzed source, which requires resolving constructor/static calls via reflection. Reflection + * must never run against a class named by the analyzed source itself unless it's one of the + * mParticle DTO/builder types (or a small set of collection helpers commonly used inline to build + * attribute maps) - anything else is attacker-controlled input, not code we intend to execute. + */ +internal object AllowedTypes { + private val ALLOWED_CLASS_NAMES = + setOf( + "com.mparticle.MPEvent", + "com.mparticle.MPEvent\$Builder", + "com.mparticle.commerce.CommerceEvent", + "com.mparticle.commerce.CommerceEvent\$Builder", + "com.mparticle.commerce.Product", + "com.mparticle.commerce.Product\$Builder", + "com.mparticle.commerce.Promotion", + "com.mparticle.commerce.Impression", + "com.mparticle.commerce.TransactionAttributes", + "com.mparticle.MParticle\$EventType", + "java.util.HashMap", + "java.util.LinkedHashMap", + "java.util.ArrayList", + "kotlin.collections.MapsKt", + "kotlin.collections.CollectionsKt", + ) + + fun isAllowed(qualifiedClassName: String?): Boolean = qualifiedClassName != null && ALLOWED_CLASS_NAMES.contains(qualifiedClassName) +} diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/Constructor.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/Constructor.kt index 1c7f5a241..af424cdd7 100644 --- a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/Constructor.kt +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/Constructor.kt @@ -7,9 +7,12 @@ import org.jetbrains.uast.UCallExpression data class Constructor(override val parent: Expression, val methodName: String?, override val node: UCallExpression) : ParameterizedExpression { override var arguments: List = listOf() - override fun resolve(): Any? { + override fun resolve(): Any? = ResolutionGuard.guarded { val qualifiedClassName = node.receiverClassName()?.replace(".Builder", "\$Builder") + if (!AllowedTypes.isAllowed(qualifiedClassName)) { + return@guarded null + } val clazz = Class.forName(qualifiedClassName) val params: List = arguments.resolve() val argumentClasses = @@ -29,15 +32,15 @@ data class Constructor(override val parent: Expression, val methodName: String?, try { if (constructor != null) { if (params.size > 0) { - return constructor.newInstance(*params.toTypedArray()) + return@guarded constructor.newInstance(*params.toTypedArray()) } else { - return constructor.newInstance() + return@guarded constructor.newInstance() } } } catch (ex: Exception) { "no new Instance for $clazz.name, tried constructor: ${constructor?.name}" } - return clazz + return@guarded clazz } override fun forEachExpression(predicate: (Expression) -> Unit) { diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/MethodCall.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/MethodCall.kt index 760743ee6..23615cf64 100644 --- a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/MethodCall.kt +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/MethodCall.kt @@ -10,10 +10,10 @@ data class MethodCall(override val parent: Expression, val methodName: String?, arguments.forEach { it.parent = this } } - override fun resolve(): Any? { + override fun resolve(): Any? = ResolutionGuard.guarded { val instance = parent.resolve() if (instance == null) { - return null + return@guarded null } var matchingMethods = instance::class.java.methods @@ -33,12 +33,12 @@ data class MethodCall(override val parent: Expression, val methodName: String?, val arguments = arguments.resolve() val value = method.invoke(instance, *arguments.toTypedArray()) if (returnValue) { - return value + return@guarded value } else { - return instance + return@guarded instance } } - return null + return@guarded null } override fun equals(other: Any?): Boolean = node.equals((other as? MethodCall)?.node) diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/ResolutionGuard.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/ResolutionGuard.kt new file mode 100644 index 000000000..e28c01dd6 --- /dev/null +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/ResolutionGuard.kt @@ -0,0 +1,25 @@ +package com.mparticle.lints.dtos + +/** + * Bounds the depth of expression resolution. Some qualified call chains cause a resolved + * expression's parent to keep resolving into itself; without a bound that recurses until the + * stack overflows, which callers' try/catch(Exception) blocks don't catch. Throwing a regular + * exception once a generous depth is exceeded keeps that failure catchable. + */ +internal object ResolutionGuard { + private const val MAX_DEPTH = 50 + private val depth = ThreadLocal.withInitial { 0 } + + fun guarded(block: () -> T): T { + val current = depth.get() + if (current >= MAX_DEPTH) { + throw IllegalStateException("Expression resolution exceeded max depth of $MAX_DEPTH") + } + depth.set(current + 1) + try { + return block() + } finally { + depth.set(current) + } + } +} diff --git a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/StaticFactory.kt b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/StaticFactory.kt index eda586ac0..73fa071be 100644 --- a/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/StaticFactory.kt +++ b/tooling/custom-lint-rules/src/main/java/com/mparticle/lints/dtos/StaticFactory.kt @@ -9,8 +9,11 @@ class StaticFactory(val methodName: String?, override val node: UCallExpression) override val parent = RootParent(node) override var arguments: List = listOf() - override fun resolve(): Any? { + override fun resolve(): Any? = ResolutionGuard.guarded { val qualifiedClassName = (node.resolve()?.parent as? ClsClassImpl)?.stub?.qualifiedName + if (!AllowedTypes.isAllowed(qualifiedClassName)) { + return@guarded null + } val methods = HashSet() val clazz = Class.forName(qualifiedClassName) methods.addAll(clazz.declaredMethods) @@ -31,9 +34,9 @@ class StaticFactory(val methodName: String?, override val node: UCallExpression) } val arguments = arguments.resolve() method.isAccessible = true - return method.invoke(null, *arguments.toTypedArray()) + return@guarded method.invoke(null, *arguments.toTypedArray()) } - return null + return@guarded null } override fun forEachExpression(predicate: (Expression) -> Unit) { diff --git a/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/Constants.kt b/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/Constants.kt index bdd888f5d..411ed2517 100644 --- a/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/Constants.kt +++ b/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/Constants.kt @@ -22,12 +22,23 @@ object Constants { @Language("JAVA") const val APPLICATION_STUB = """ - package android.app; + package android.app; public class Application { public void onCreate() {} public void onResume() {} }""" + @Language("JAVA") + const val MPEVENT_STUB = """package com.mparticle; + public class MPEvent { + public static class Builder { + public Builder(String eventName) {} + public Builder customAttributes(java.util.Map customAttributes) { return this; } + public MPEvent build() { return new MPEvent(); } + } + }""" + val mParticleStubClass = java(MPARTICLE_STUB) val mApplicationStubClass = java(APPLICATION_STUB) + val mpEventStubClass = java(MPEVENT_STUB) } diff --git a/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/DataplanDetectorTest.kt b/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/DataplanDetectorTest.kt index dabf4c961..c474e6e03 100644 --- a/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/DataplanDetectorTest.kt +++ b/tooling/custom-lint-rules/src/test/java/com/mparticle/lints/DataplanDetectorTest.kt @@ -2,9 +2,14 @@ package com.mparticle.lints import com.android.tools.lint.checks.infrastructure.LintDetectorTest import com.android.tools.lint.checks.infrastructure.TestMode +import com.mparticle.lints.Constants.mApplicationStubClass import com.mparticle.lints.Constants.mParticleStubClass +import com.mparticle.lints.Constants.mpEventStubClass import com.mparticle.lints.detectors.DataplanDetector +import com.mparticle.tooling.Config +import com.mparticle.tooling.Utils import org.intellij.lang.annotations.Language +import org.junit.Assert.assertTrue import org.junit.Test import java.io.File @@ -35,6 +40,97 @@ class DataplanDetectorTest : LintDetectorTest() { .expectErrorCount(0) } + // Regression test: a local variable feeding an MPEvent.Builder argument used to be resolved + // by reflectively replaying every call made on it in the enclosing method, with no + // restriction on what classes/methods those calls could touch. This reproduces that shape + // (an unrelated File operation feeding the event name) and asserts lint never performs it. + @Test + fun testEnclosingMethodSideEffectsAreNeverExecuted() { + val sdkHome = + System.getenv("ANDROID_HOME") + ?: "${System.getProperty("user.home")}/Library/Android/sdk" + + val marker = File.createTempFile("mparticle-lint-regression", ".txt") + marker.writeText("must survive lint analysis") + val markerPath = marker.absolutePath.replace("\\", "\\\\") + + try { + withConfigFile(Config()) { + @Language("JAVA") + val source = """ + package com.mparticle.lints; + import android.app.Application; + import com.mparticle.MPEvent; + import java.io.File; + public class HasUnrelatedFileCall extends Application { + @Override + public void onCreate() { + super.onCreate(); + File f = new File("$markerPath"); + f.delete(); + String name = f.getName(); + new MPEvent.Builder(name); + } + } + """ + lint() + .sdkHome(File(sdkHome)) + .files(java(source), mParticleStubClass, mApplicationStubClass, mpEventStubClass) + .skipTestModes(TestMode.PARENTHESIZED) + .run() + } + assertTrue( + "Data plan lint must not perform filesystem operations found in analyzed source", + marker.exists(), + ) + } finally { + marker.delete() + } + } + + // A legitimate builder chain (the only thing this detector needs to resolve) must keep + // resolving successfully - the allowlist should reject non-DTO reflection, not the feature. + @Test + fun testAllowlistedBuilderChainStillResolves() { + val sdkHome = + System.getenv("ANDROID_HOME") + ?: "${System.getProperty("user.home")}/Library/Android/sdk" + + withConfigFile(Config()) { + @Language("JAVA") + val source = """ + package com.mparticle.lints; + import android.app.Application; + import com.mparticle.MPEvent; + public class HasAllowlistedCall extends Application { + @Override + public void onCreate() { + super.onCreate(); + new MPEvent.Builder("test").build(); + } + } + """ + val result = + lint() + .sdkHome(File(sdkHome)) + .files(java(source), mParticleStubClass, mApplicationStubClass, mpEventStubClass) + .skipTestModes(TestMode.PARENTHESIZED) + .run() + // With no local data plan configured, a successfully-resolved event is reported as + // NO_DATA_PLAN; a failure to resolve at all would instead produce no report. + result.expectContains(DataplanDetector.NO_DATA_PLAN.id) + } + } + + private fun withConfigFile(config: Config, block: () -> Unit) { + Utils.setConfigFile(config) + try { + block() + } finally { + Utils.removeConfigFile() + } + } + override fun requireCompileSdk() = false override fun getDetector() = DataplanDetector() From 1d6377b7454e729f77618f0ad7776c8c3e2be9e7 Mon Sep 17 00:00:00 2001 From: Nickolas Dimitrakas Date: Thu, 17 Sep 2026 10:35:50 -0400 Subject: [PATCH 2/2] docs: link CHANGELOG entry to PR #817 Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6b041a7f..1c1d5676a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ #### Security -- Restrict the Data Plan lint detector to only ever construct or invoke mParticle event/commerce builder types when evaluating analyzed source, instead of reflecting on any class named in a local variable's initializer chain ([#TBD](https://github.com/mParticle/mparticle-android-sdk/pull/TBD)) +- Restrict the Data Plan lint detector's expression evaluation to an explicit allowlist of mParticle event/commerce builder types ([#817](https://github.com/mParticle/mparticle-android-sdk/pull/817)) ## [6.1.0] - 2026-09-15