False positive "use of implicit PendingIntents" alert

[tmo1]

Description of the false positive

CodeQL reports a "use of implicit PendingIntents," but the Intent in question seems to actually be explicit.

Code samples or links to source code

https://github.com/tmo1/sms-ie/blob/bad53787c0bcf1f4ec5fc18e4ed6623a756373d9/app/src/main/java/com/github/tmo1/sms_ie/ImportExportWorker.kt#L364-L364

URL to the alert on GitHub code scanning (optional)

tmo1/sms-ie#289 (review)

[hvitved]

Thank you for this false positive report. Resolving this issue is not a current product priority, but we acknowledge the report and will track it internally for future consideration, or if we observe repeated instances of the same problem.

[david-allison]

Reproduced in ankidroid/Anki-Android PR 20976 [permalink]. Alert #35 (now marked as a false positive). There's 2 bugs, both caused by changes introduced by Kotlin 2.0 (K2 compiler) fixing either of which will stop this false positive.

Ref: ImplicitPendingIntents.qll

• Fixing (A) properly appears to require a change to semmle to extract the ConstantValue of fields in a .class to the CodeQL database
  • K2 replaces FLAG_IMMUTABLE with the constant 67108864
  • We need to know that FLAG_IMMUTABLE == 67108864, but this isn't in the DB
  • Hardcoding a couple of constants would be feasible in the interim
  • NOTE: Avoiding storing field values may be intentional for performance.
• Fixing (B) seemed feasible: Kotlin: Fix findTopLevelPropertyOrWarn for K2 compiler #21915

Using github/codeql 2.25.5 (a16f1c5)

This is flagged, but:

• Explicit intent makes this safe: CopyToClipboardReceiver::class.java
• Immutable flag makes this safe: or PendingIntent.FLAG_IMMUTABLE

val intent = Intent(applicationContext, CopyToClipboardReceiver::class.java)
intent.apply { putExtra(CopyToClipboardReceiver.SYNC_ERROR_LOG, text) }
return PendingIntent.getBroadcast(
    applicationContext, 0, intent,
    PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE,
)
// attached as a notification action and dispatched via NotificationManagerCompat.notify

Root causes

A) K2 folds compile-time constant operands (static final int)

1. FLAG_IMMUTABLE declared as static final int:

public static final int FLAG_IMMUTABLE = 1<<26;

https://cs.android.com/android/_/android/platform/frameworks/base/+/7c65c003a1f93d801914835b874e12cd66d2f5a3:core/java/android/app/PendingIntent.java;l=269;bpv=1;bpt=0

(in android.jar)

public static final int FLAG_IMMUTABLE;
  descriptor: I
  flags: (0x0019) ACC_PUBLIC, ACC_STATIC, ACC_FINAL
  ConstantValue: int 67108864

2. Kotlin language version >= 2.0

K2 folds compile-time constant operands

---

K2 using IntegerLiteral 134217728; kotlinc 1.9 using VarAccess FLAG_IMMUTABLE

codeql/java/ql/lib/semmle/code/java/security/ImplicitPendingIntents.qll

Lines 86 to 106 in a16f1c5

	// We err on the side of false positives here, assuming a PendingIntent may be mutable
	// unless it is at least sometimes explicitly marked immutable and never marked mutable.
	// Note: for API level < 31, PendingIntents were mutable by default, whereas since then
	// they are immutable by default.
	not bitwiseLocalTaintStep*(DataFlow::exprNode(any(ImmutablePendingIntentFlag flag)
	.getAnAccess()), DataFlow::exprNode(flagArg))
	or
	bitwiseLocalTaintStep*(DataFlow::exprNode(any(MutablePendingIntentFlag flag).getAnAccess()),
	DataFlow::exprNode(flagArg))
	)
	}
	}
	/**
	* Holds if taint can flow from `source` to `sink` in one local step,
	* including bitwise operations.
	*/
	private predicate bitwiseLocalTaintStep(DataFlow::Node source, DataFlow::Node sink) {
	TaintTracking::localTaintStep(source, sink) or
	source.asExpr() = sink.asExpr().(BitwiseExpr).(BinaryExpr).getAnOperand()
	}

sink.asExpr().(BitwiseExpr).(BinaryExpr).getAnOperand() no longer works under K2, as it relies on FLAG_IMMUTABLE remaining an operand of the bitwise expression, but it became an IntegerLiteral

flags decl	Kotlin lang ver	flags-arg AST	FLAG_IMMUTABLE access	query
static int (current test stub)	1.9	OrBitwiseExpr(VarAccess FLAG_IMMUTABLE, VarAccess FLAG_UPDATE_CURRENT)	present	safe
static final int (real values)	1.9	OrBitwiseExpr(VarAccess FLAG_IMMUTABLE, VarAccess FLAG_UPDATE_CURRENT)	present	safe
static final int	2.0 / 2.3	OrBitwiseExpr(IntegerLiteral 67108864, IntegerLiteral 134217728)	count 0	False Positive

After folding, the flags arg is itself a CompileTimeConstantExpr with getIntValue() = 201326592 (= 134217728 | 67108864), but the FLAG_IMMUTABLE field access has been erased from the IR. This means we can't map 67108864 back to FLAG_IMMUTABLE.

I don't believe CodeQL exposes ConstantValue for .class files inside android.jar:

QL:
Field.getInitializer() resolves only source-level initializer expressions; library fields have none.

Extractor: conjecture

Fields can be stored, but I do not believe the fields of .jar files are

	initializer literal emitted?	value in DB?
source field X = 67108864	yes (IntegerLiteral)	yes
library field (the jar)	no	no

B) K2 - ::class.java arg dropped

An extraction of Intent(ctx, Foo::class.java) produces:

• Kotlin 1.9: a call with 2 args
• K2 (Kotlin 2.0+): a call with 1 arg - Foo::class.java is dropped.

  sev=5  @GapA.kt:9  ::  Couldn't find JVM intrinsic property kotlin.jvm java in kotlin.jvm.JvmClassMappingKt
    ...while extracting a call (<no name>)        at GapA.kt:9:61:9:64
    ...while extracting a expression (<no name>)  at GapA.kt:9:61:9:64
    ...while extracting a expression (<no name>)  at GapA.kt:9:35:9:65
    ...while extracting a statement (<no name>)   at GapA.kt:9:66:9:65
    ...while extracting a block body              at GapA.kt:9:35:9:65
    ...while extracting a body                    at GapA.kt:9:35:9:65
    ...while extracting a function (f)            at GapA.kt:9:5:9:65
    ...while extracting a class source (GapA)     at GapA.kt:8:1:10:1
    ...while extracting a file (GapA.kt)          at GapA.kt:1:1:11:0

• See Kotlin: Fix findTopLevelPropertyOrWarn for K2 compiler #21915 for a proposed fix

Resolutions

A)

1. [hack] Hardcode the values in the query: FLAG_IMMUTABLE = 1 << 26, FLAG_MUTABLE = 1 << 25
   • These are effectively constant in the Android ABI
2. Expose ConstantValue in the semmle-code Java extractor
   • Ideal fix, if this wasn't explicitly avoided due to performance problems

B)

• Kotlin: Fix findTopLevelPropertyOrWarn for K2 compiler #21915