Add Fragment and Activity edge case

Author: atorralba

java/ql/lib/semmle/code/java/frameworks/android/OnActivityResultSource.qll

@@ -28,10 +28,26 @@ class OnActivityResultIncomingIntent extends DataFlow::Node {
    * Intent to `onActivityResult`.
    */
   predicate isRemoteSource() {
-    exists(ImplicitStartActivityForResultConf conf, DataFlow::Node sink |
+    exists(ImplicitStartActivityForResultConf conf, RefType startingType, DataFlow::Node sink |
       conf.hasFlowTo(sink) and
-      DataFlow::getInstanceArgument(sink.asExpr().(Argument).getCall()).getType() =
-        this.getEnclosingCallable().getDeclaringType()
+      startingType = sink.asExpr().(Argument).getCall().getEnclosingCallable().getDeclaringType()
+    |
+      startingType = this.getEnclosingCallable().getDeclaringType()
+      or
+      // A fragment calls `startActivityForResult`
+      // and the activity it belongs to defines `onActivityResult`.
+      exists(MethodAccess ma |
+        ma.getMethod().hasName(["add", "attach", "replace"]) and
+        ma.getMethod().getDeclaringType().hasName("FragmentTransaction") and
+        any(Argument arg | arg = ma.getAnArgument()).getType() = startingType
+        or
+        ma.getMethod().hasName("show") and
+        ma.getMethod().getDeclaringType().getASupertype*().hasName("DialogFragment") and
+        startingType = ma.getQualifier().getType()
+      |
+        ma.getEnclosingCallable().getDeclaringType() =
+          this.getEnclosingCallable().getDeclaringType()
+      )
     )
   }
 }
@@ -49,16 +65,38 @@ private class ImplicitStartActivityForResultConf extends DataFlow5::Configuratio
   }
 
   override predicate isSink(DataFlow::Node sink) {
-    exists(ActivityOrFragment actOrFrag, MethodAccess startActivityForResult |
+    exists(MethodAccess startActivityForResult |
       startActivityForResult.getMethod().hasName("startActivityForResult") and
-      startActivityForResult.getEnclosingCallable() = actOrFrag.getACallable() and
+      startActivityForResult.getMethod().getDeclaringType().getASupertype*() instanceof
+        ActivityOrFragment and
       sink.asExpr() = startActivityForResult.getArgument(0)
     )
   }
 
   override predicate isBarrier(DataFlow::Node barrier) {
     barrier instanceof ExplicitIntentSanitizer
   }
+
+  override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    // Wrapping the Intent in a chooser
+    exists(MethodAccess ma, Method m |
+      ma.getMethod() = m and
+      m.hasName("createChooser") and
+      m.getDeclaringType() instanceof TypeIntent
+    |
+      node1.asExpr() = ma.getArgument(0) and
+      node2.asExpr() = ma
+    )
+    or
+    // Using the copy constructor
+    exists(ClassInstanceExpr cie |
+      cie.getConstructedType() instanceof TypeIntent and
+      cie.getArgument(0).getType() instanceof TypeIntent
+    |
+      node1.asExpr() = cie.getArgument(0) and
+      node2.asExpr() = cie
+    )
+  }
 }
 
 /** An Android Activity or Fragment. */

java/ql/test/library-tests/frameworks/android/sources/TestActivityAndFragment.java

@@ -0,0 +1,32 @@
+import android.app.Activity;
+import android.app.Fragment;
+import android.content.Intent;
+import android.os.Bundle;
+import androidx.fragment.app.FragmentTransaction;
+
+public class TestActivityAndFragment extends Activity {
+
+    private TestFragment frag;
+
+    void sink(Object o) {}
+
+    public void onCreate(Bundle saved) {
+        FragmentTransaction ft = null;
+        ft.add(0, frag);
+
+    }
+
+    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
+        sink(requestCode); // safe
+        sink(resultCode); // safe
+        sink(data); // $ hasValueFlow
+    }
+
+    private class TestFragment extends Fragment {
+        public void onCreate(Bundle savedInstance) {
+            Intent implicitIntent = new Intent("SOME_ACTION");
+            startActivityForResult(implicitIntent, 0);
+        }
+
+    }
+}