From 46674338cdbeb35362d4bf3bb14b53b1f09a197c Mon Sep 17 00:00:00 2001
From: Maxime Arthaud <arthaud@meta.com>
Date: Thu, 10 Aug 2023 07:09:02 -0700
Subject: [PATCH] Error out on more invalid model syntax

Summary: I found out a few other cases where we silently parse an invalid taint annotation. Let's fix it.

Reviewed By: dkgi

Differential Revision: D48202458

fbshipit-source-id: 5567842ae3a446348153b8c166a2685b1197bcb2
---
 .../taint/modelParser.ml                        |  9 +++++----
 .../taint/test/modelTest.ml                     | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/source/interprocedural_analyses/taint/modelParser.ml b/source/interprocedural_analyses/taint/modelParser.ml
index 32a1d641ff1..b3baa96ad2c 100644
--- a/source/interprocedural_analyses/taint/modelParser.ml
+++ b/source/interprocedural_analyses/taint/modelParser.ml
@@ -287,14 +287,16 @@ let rec parse_annotations
         }
       when is_base_name callee "WithTag" ->
         Ok (Some value)
-    | Expression.Call _ ->
+    | Expression.Name (Name.Identifier _) when not is_object_target ->
+        (* This should be the parameter name. *)
+        Ok None
+    | Tuple expressions -> List.map expressions ~f:extract_via_tag |> all >>| List.find_map ~f:ident
+    | _ ->
         Error
           (annotation_error
              (Format.sprintf
                 "Invalid expression in ViaValueOf or ViaTypeOf declaration: %s"
                 (Expression.show expression)))
-    | Tuple expressions -> List.map expressions ~f:extract_via_tag |> all >>| List.find_map ~f:ident
-    | _ -> Ok None
   in
   let rec extract_names expression =
     match expression.Node.value with
@@ -334,7 +336,6 @@ let rec parse_annotations
         | "Collapse" -> Ok (TaintKindsWithFeatures.from_collapse_depth CollapseDepth.Collapse)
         | "NoCollapse" -> Ok (TaintKindsWithFeatures.from_collapse_depth CollapseDepth.NoCollapse)
         | taint_kind -> Ok (TaintKindsWithFeatures.from_kind (Kind.from_name taint_kind)))
-    | Name (Name.Attribute { base; _ }) -> extract_kinds_with_features base
     | Call { callee; arguments = [{ Call.Argument.value = argument; _ }] } -> (
         match base_name callee with
         | Some "Via" -> extract_breadcrumbs argument >>| TaintKindsWithFeatures.from_breadcrumbs
diff --git a/source/interprocedural_analyses/taint/test/modelTest.ml b/source/interprocedural_analyses/taint/test/modelTest.ml
index eeb0d30a2d9..aa3a9e2f6d0 100644
--- a/source/interprocedural_analyses/taint/test/modelTest.ml
+++ b/source/interprocedural_analyses/taint/test/modelTest.ml
@@ -2643,6 +2643,23 @@ let test_invalid_models context =
       "`TaintSource[WithSubkind[A][B]]` is an invalid taint annotation: Invalid expression for \
        taint kind: WithSubkind[A][B]"
     ();
+  assert_invalid_model
+    ~source:"def f(x: int): ..."
+    ~model_source:"def test.f(x) -> TaintSource[Test.attribute]: ..."
+    ~expect:
+      "`TaintSource[Test.attribute]` is an invalid taint annotation: Invalid expression for taint \
+       kind: Test.attribute"
+    ();
+  assert_invalid_model
+    ~source:{|
+      class C:
+        x: int = 0
+      |}
+    ~model_source:"test.C.x: ViaTypeOf[a.b] = ..."
+    ~expect:
+      "`ViaTypeOf[a.b]` is an invalid taint annotation: Invalid expression in ViaValueOf or \
+       ViaTypeOf declaration: a.b"
+    ();
   ();
 
   (* Test invalid model queries. *)