Skip to content

Commit

Permalink
Correctly bind a metavariable in an import to the fully qualified name
Browse files Browse the repository at this point in the history
Fixes #1771

This should also help
#975

Test plan:

$ semgrep -f /tmp/test2.yml tests/python/import_metavar_fullpath.py
running 1 rules...
tests/python/import_metavar_fullpath.py
severity:error rule:tmp.import-auth: spooky import

12:import a.auth
14:import b.auth
16:import a.b.auth
ran 1 rules on 1 files: 3 findings
  • Loading branch information
aryx committed Nov 19, 2020
1 parent 60e4c58 commit 04bdde9
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 14 deletions.
45 changes: 31 additions & 14 deletions semgrep-core/matching/Generic_vs_generic.ml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,17 @@ let has_xml_ellipis_and_filter_ellispis xs =
| _ -> false) in
!has_ellipsis, ys

(*s: function [[Generic_vs_generic.make_dotted]] *)
let make_dotted xs =
match xs with
| [] -> raise Impossible
| x::xs ->
let base = B.Id (x, B.empty_id_info()) in
List.fold_left (fun acc e ->
let tok = Parse_info.fake_info "." in
B.DotAccess (acc, tok, B.EId e)) base xs
(*e: function [[Generic_vs_generic.make_dotted]] *)

(*****************************************************************************)
(* Name *)
(*****************************************************************************)
Expand Down Expand Up @@ -155,6 +166,21 @@ let m_dotted_name a b =
(a, b) -> (m_list m_ident) a b
(*e: function [[Generic_vs_generic.m_dotted_name]] *)

(* similar to m_list_prefix but binding $X to the whole list *)
let rec m_dotted_name_prefix_ok a b =
match a, b with
| [], [] -> return ()
| [(s,t)], _::_ when MV.is_metavar_name s ->
envf (s,t) (B.E (make_dotted b))
| xa::aas, xb::bbs ->
let* () = m_ident xa xb in
m_dotted_name_prefix_ok aas bbs
(* prefix is ok *)
| [], _ -> return ()

| _::_, _ ->
fail ()


(*s: function [[Generic_vs_generic.m_module_name_prefix]] *)
(* less-is-ok: prefix matching is supported for imports, eg.:
Expand All @@ -165,8 +191,8 @@ let m_module_name_prefix a b =
(* metavariable case *)
| A.FileName((a_str, _) as a1), B.FileName(b1) when MV.is_metavar_name a_str ->
(* Bind as a literal string expression so that pretty-printing works.
* This also means that this metavar can match both literal strings and filenames
* with the same string content. *)
* This also means that this metavar can match both literal strings and
* filenames with the same string content. *)
envf a1 (B.E (B.L (B.String b1)))

(* dots: '...' on string *)
Expand All @@ -183,8 +209,10 @@ let m_module_name_prefix a b =
| A.FileName(a1), B.FileName(b1) ->
(* TODO figure out what prefix support means here *)
(m_wrap m_string_prefix) a1 b1

| A.DottedName(a1), B.DottedName(b1) ->
m_list_prefix m_ident a1 b1
m_dotted_name_prefix_ok a1 b1

| A.FileName _, _
| A.DottedName _, _
-> fail ()
Expand Down Expand Up @@ -367,17 +395,6 @@ and m_id_info a b =
(* Expression *)
(*****************************************************************************)

(*s: function [[Generic_vs_generic.make_dotted]] *)
and make_dotted xs =
match xs with
| [] -> raise Impossible
| x::xs ->
let base = B.Id (x, B.empty_id_info()) in
List.fold_left (fun acc e ->
let tok = Parse_info.fake_info "." in
B.DotAccess (acc, tok, B.EId e)) base xs
(*e: function [[Generic_vs_generic.make_dotted]] *)

(* possibly go deeper when someone wants that a pattern like
* 'bar();'
* match also an expression statement like
Expand Down
2 changes: 2 additions & 0 deletions semgrep-core/matching/Matching_generic.ml
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,9 @@ let rec m_list_prefix f a b =
f xa xb >>= (fun () ->
m_list_prefix f aas bbs
)
(* less-is-ok: prefix is ok *)
| [], _ -> return ()

| _::_, _ ->
fail ()
(*e: function [[Matching_generic.m_list_prefix]] *)
Expand Down
16 changes: 16 additions & 0 deletions semgrep-core/tests/python/import_metavar_fullpath.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# what actually matters in this test is the value binded to $X
# in the .sgrep. It should be the full path!
# use semgrep-core ... -pvar '$X' to print the value.

#ERROR: match
import a
#ERROR: match
import auth
#ERROR: match
import arbitrary_auth
#ERROR: match
import a.auth
#ERROR: match
import b.auth
#ERROR: match
import a.b.auth
1 change: 1 addition & 0 deletions semgrep-core/tests/python/import_metavar_fullpath.sgrep
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import $X

0 comments on commit 04bdde9

Please sign in to comment.