Add user-declared breadcrumbs as local features of the first frame

Summary: # Problem Breadcrumbs directly attached to user-declared taint (for instance, using `TaintSource[Test, Via[foo]]`) appear in the issue but do NOT appear on any frame. This is because those are stored as **propagated** breadcrumbs on the "declaration" frame, which is not actually exported to SAPP. Only the "origin" frame (i.e first frame) is exported. See also the test D63631358. # Solution We need custom logic to mark those **propagated** breadcrumbs of "declaration" frames as local breadcrumbs on the first frame (i.e the "Origin" frame). This can be done easily during the "apply_call" logic. Reviewed By: tianhan0 Differential Revision: D63632529 fbshipit-source-id: a56471a008b7cef18543bd92379ed1cd7eb8b878
facebook · Sep 30, 2024 · a744211 · a744211
1 parent 8644d1c
commit a744211
Show file tree

Hide file tree

Showing 85 changed files with 1,337 additions and 1,323 deletions.
diff --git a/source/interprocedural_analyses/taint/domains.ml b/source/interprocedural_analyses/taint/domains.ml
@@ -1299,22 +1299,36 @@ end = struct
         |> LocalTaintDomain.update LocalTaintDomain.Slots.FirstField Features.FirstFieldSet.bottom
       in
       let apply_frame frame =
-        let frame =
+        let frame, kind_specific_local_breadcrumbs =
           match call_info with
           | CallInfo.Declaration _ ->
-              (* Even if we allow extra traces on user models in the future, we would still want to
-                 propagate them to the first frame. This is because the Declaration frame is never
-                 shown in the Zoncolan UI. The first frame is the Origin one. *)
-              frame
-          | _ -> Frame.update Frame.Slots.ExtraTraceFirstHopSet ExtraTraceFirstHop.Set.bottom frame
+              (* Propagate breadcrumbs and extra traces onto the first frame, since Declaration
+                 frame are never shown in the SAPP UI. The first frame is the Origin one. *)
+              let user_declared_breadcrumbs = Frame.get Frame.Slots.PropagatedBreadcrumb frame in
+              let frame =
+                frame
+                |> Frame.transform
+                     Features.LocalKindSpecificBreadcrumbSet.Self
+                     Map
+                     ~f:(Features.BreadcrumbSet.add_set ~to_add:user_declared_breadcrumbs)
+                |> Frame.update Frame.Slots.PropagatedBreadcrumb Features.BreadcrumbSet.empty
+              in
+              frame, Features.BreadcrumbSet.empty
+          | _ ->
+              let local_breadcrumbs = Frame.get Frame.Slots.LocalKindSpecificBreadcrumb frame in
+              let frame =
+                frame
+                |> Frame.update Frame.Slots.ExtraTraceFirstHopSet ExtraTraceFirstHop.Set.bottom
+                |> Frame.update Frame.Slots.LocalKindSpecificBreadcrumb Features.BreadcrumbSet.empty
+              in
+              frame, local_breadcrumbs
         in
+        (* Existing local breadcrumbs (kind specific or not) become "propagated" breadcrumbs *)
         let local_breadcrumbs =
-          Frame.get Frame.Slots.LocalKindSpecificBreadcrumb frame
-          |> Features.BreadcrumbSet.sequence_join local_breadcrumbs
+          Features.BreadcrumbSet.sequence_join kind_specific_local_breadcrumbs local_breadcrumbs
         in
         frame
         |> Frame.update Frame.Slots.ViaFeature Features.ViaFeatureSet.bottom
-        |> Frame.update Frame.Slots.LocalKindSpecificBreadcrumb Features.BreadcrumbSet.empty
         |> Frame.transform
              Features.PropagatedBreadcrumbSet.Self
              Map

diff --git a/source/interprocedural_analyses/taint/test/integration/access_path.py.models b/source/interprocedural_analyses/taint/test/integration/access_path.py.models
@@ -17,7 +17,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -42,7 +42,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -90,7 +90,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -115,7 +115,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -163,7 +163,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -188,7 +188,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -236,7 +236,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -308,7 +308,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -380,7 +380,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -557,7 +557,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -587,7 +587,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }

diff --git a/source/interprocedural_analyses/taint/test/integration/add_feature_to_argument.py.models b/source/interprocedural_analyses/taint/test/integration/add_feature_to_argument.py.models
@@ -17,7 +17,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -44,7 +44,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -138,7 +138,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_lhs" } ],
+                "local_features": [ { "always-via": "string_concat_lhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_first",
@@ -253,7 +253,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -286,7 +286,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -324,7 +324,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_lhs" } ],
+                "local_features": [ { "always-via": "string_concat_lhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_first",
@@ -359,7 +359,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_rhs" } ],
+                "local_features": [ { "always-via": "string_concat_rhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_second",
@@ -384,7 +384,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_lhs" } ],
+                "local_features": [ { "always-via": "string_concat_lhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_first",
@@ -419,7 +419,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],
@@ -457,7 +457,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_lhs" } ],
+                "local_features": [ { "always-via": "string_concat_lhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_first",
@@ -477,7 +477,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -508,7 +508,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "string_concat_lhs" } ],
+                "local_features": [ { "always-via": "string_concat_lhs" } ],
                 "leaves": [
                   {
                     "name": "add_feature_to_argument.add_feature_to_first",
@@ -528,7 +528,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -559,7 +559,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -644,7 +644,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_source" } ],
+                "local_features": [ { "always-via": "special_source" } ],
                 "leaves": [
                   { "name": "_test_source", "port": "leaf:return" }
                 ],

diff --git a/source/interprocedural_analyses/taint/test/integration/always_in_none.py.models b/source/interprocedural_analyses/taint/test/integration/always_in_none.py.models
@@ -93,7 +93,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }
@@ -182,7 +182,7 @@
           {
             "kinds": [
               {
-                "features": [ { "always-via": "special_sink" } ],
+                "local_features": [ { "always-via": "special_sink" } ],
                 "leaves": [ { "name": "_test_sink", "port": "leaf:arg" } ],
                 "kind": "Test"
               }