(Rebased) Java android remote source

[smowton]

Just a rebased version of #3812 to make reviewing easier; direct all comments over there.

[smowton]

Can we push this isExported() filter right into AndroidIntentInput, since that seems like the point where we know whether the source is dangerous or not

[luchua-bc]

Moved the the isExported() check to AndroidIntentInput.

[smowton]

Probably not necessary -- once AndroidIntentInput is reduced to only inputs of exported classes, we can simply have that class extend RemoteFlowSource

[luchua-bc]

Yes, it's not required once AndroidIntentInput is reduced to only inputs of exported classes. Changed AndroidIntentInput to extend RemoteFlowSource.

[smowton]

Split these two (get***Extra and the Bundle getters), since they're pretty unrelated

[luchua-bc]

With the AndroidIntentInput change, only Bundle getters modelled in GetBundleExtraMethodAccess are required as the additional taint step.

[smowton]

Has conveying taint from the Intent to the Bundle yielded by getExtra gone missing? I'm seeing steps for intent -> getStringExtra and for Bundle -> getString but not for intent -> getExtra -> Bundle -> getString?

[luchua-bc]

I've tested all flow combinations including intent -> getExtras -> Bundle -> getString and the query seems to work as expected.

Do I still miss something?

[joefarebrother]

@luchua-bc Regarding your question on TaintPreservingCallable on the original PR, where were you trying to define it? Note that if it was outside of FlowSteps.qll (which is recommended), you need to ensure that the Frameworks module in FlowSteps imports it, to make the definition visible to all queries.

Also note that getExtra methods on Intent are already modelled in android.Intent.qll, so only the methods on Bundle are necassary.

[luchua-bc]

Thanks @joefarebrother for reviewing the PR. I'm trying to detect a pattern like the following:

public class UnsafeAndroidActivity extends Activity {
	public void onCreate(Bundle savedInstanceState) {
		Intent intent = getIntent();
		Bundle bundle = intent.getExtras();
		String thisUrl = bundle.getString("url");
		WebView wv = (WebView) findViewById(-1);
		wv.loadUrl(thisUrl);
	}
}

The flow is getIntent() -> intent -> getExtras() -> bundle -> getString("url").

I thought we must put new TaintPreservingCallable implementations like BundleExtraTaintPreservingCallable in FlowSteps.qll so that all queries can use them. What's the desired location for BundleExtraTaintPreservingCallable?

Thanks,
@luchua-bc

[luchua-bc]

Added TaintPreservingCallable implementation to java/ql/src/semmle/code/java/frameworks/android/Android.qll instead of FlowSteps.qll