Skip to content

Commit

Permalink
[dockerfile] Fix named ellipsis matching on CMD-like instructions usi…
Browse files Browse the repository at this point in the history
…ng the array syntax (semgrep/semgrep-proprietary#2310)

See description in the changelog.

Fixes #9726

The tests use an updated semgrep-rules where two TODOs were promoted to
expectations. The PR is here:
semgrep/semgrep-rules#3477

synced from Pro 1743a2736e2153e4210516c31ebdbbfcefd310e2
  • Loading branch information
mjambon authored and ajbt200128 committed Sep 26, 2024
1 parent f96b3ce commit e596f43
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 7 deletions.
4 changes: 4 additions & 0 deletions changelog.d/gh-9726.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Dockerfile matching: `CMD $...ARGS` now behaves like `CMD ...` and matches
any CMD instruction that uses the array syntax such as `CMD ["ls"]`. This
fix also applies to the other command-like instructions RUN
and ENTRYPOINT.
1 change: 1 addition & 0 deletions languages/dockerfile/ast/AST_dockerfile.ml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ type command =
| Sh_command of loc * AST_bash.blist
| Other_shell_command of shell_compatibility * string wrap
| Command_semgrep_ellipsis of tok
| Command_semgrep_named_ellipsis of string wrap
(* If the instruction contains at least one heredoc template, we don't
know the shell command statically and we produce such a template.
Only the RUN instruction supports heredocs in commands, not CMD,
Expand Down
1 change: 1 addition & 0 deletions languages/dockerfile/ast/AST_dockerfile_loc.ml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ let _quoted_string_loc = bracket_loc
*)
let command_loc = function
| Command_semgrep_ellipsis tok -> (tok, tok)
| Command_semgrep_named_ellipsis x -> wrap_loc x
| Argv (loc, _) -> loc
| Sh_command (loc, _) -> loc
| Other_shell_command (_, x) -> wrap_loc x
Expand Down
1 change: 1 addition & 0 deletions languages/dockerfile/generic/Dockerfile_to_generic.ml
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ let string_array ((open_, args, close) : string_array) : G.expr =
let command (env : env) (x : command) : G.expr list =
match x with
| Command_semgrep_ellipsis tok -> [ G.Ellipsis tok |> G.e ]
| Command_semgrep_named_ellipsis x -> [ id_expr x ]
| Argv (_loc, array) -> [ string_array array ]
| Sh_command (loc, x) ->
(* !!! Calling Bash_to_generic !!! *)
Expand Down
21 changes: 18 additions & 3 deletions languages/dockerfile/tree-sitter/Parse_dockerfile_tree_sitter.ml
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ let remove_blank_prefix (x : string wrap) : string wrap =

type ellipsis_or_bash =
| Semgrep_ellipsis of tok
| Semgrep_named_ellipsis of string wrap
| Bash of AST_bash.blist option

(* A plain ellipsis such as '...' (not e.g. '...;') is identified so
Expand All @@ -221,7 +222,18 @@ type ellipsis_or_bash =
grammar.
*)
let is_plain_ellipsis =
let rex = Pcre2_.regexp "\\A[ \t\r\n]*[.]{3}[ \t\r\n]*\\z" in
let rex = Pcre2_.regexp {|\A[ \t\r\n]*[.]{3}[ \t\r\n]*\z|} in
fun s ->
match Pcre2_.pmatch ~rex s with
| Ok res -> res
| Error _err -> false

(* A named ellipsis such as '$...ARGS'.
See remarks for 'is_plain_ellipsis'. *)
let is_named_ellipsis =
let rex =
Pcre2_.regexp {|\A[ \t\r\n]*(\$[.]{3}[A-Z_][A-Z_0-9]*)[ \t\r\n]*\z|}
in
fun s ->
match Pcre2_.pmatch ~rex s with
| Ok res -> res
Expand All @@ -236,9 +248,11 @@ let shift_locations (str, tok) =

let parse_bash (env : env) shell_cmd : ellipsis_or_bash =
let input_kind, _ = env.extra in
let contents, tok = shell_cmd in
match input_kind with
| Pattern when is_plain_ellipsis (fst shell_cmd) ->
Semgrep_ellipsis (snd shell_cmd)
| Pattern when is_plain_ellipsis contents -> Semgrep_ellipsis (snd shell_cmd)
| Pattern when is_named_ellipsis contents ->
Semgrep_named_ellipsis (String.trim contents, tok)
| _ ->
let ts_res =
H.wrap_parser
Expand Down Expand Up @@ -820,6 +834,7 @@ let shell_command (env : env) (x : CST.shell_command) =
| Sh -> (
match parse_bash env raw_shell_code with
| Semgrep_ellipsis tok -> Command_semgrep_ellipsis tok
| Semgrep_named_ellipsis x -> Command_semgrep_named_ellipsis x
| Bash (Some bash_program) ->
let loc = wrap_loc raw_shell_code in
Sh_command (loc, bash_program)
Expand Down
13 changes: 10 additions & 3 deletions src/engine/tests/Test_compare_matches.ml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ let (expected_error_lines_of_files :
(* Entry points *)
(*****************************************************************************)

let plural n = if n >= 2 then "s" else ""

let compare_actual_to_expected ~to_location actual_findings
expected_findings_lines =
let actual_findings = List_.map to_location actual_findings in
Expand All @@ -71,10 +73,15 @@ let compare_actual_to_expected ~to_location actual_findings
only_in_actual
|> List.iter (fun (src, l) ->
UCommon.pr2 (spf "this one finding was not expected: %s:%d" !!src l));
let num_errors = List.length only_in_actual + List.length only_in_expected in
let false_negatives = List.length only_in_expected in
let false_positives = List.length only_in_actual in
let num_errors = false_negatives + false_positives in
let msg =
spf "it should find all reported findings and no more (%d errors)"
num_errors
spf
"The test failed to find exactly the expected findings: %i false \
negative%s, %i false positive%s."
false_negatives (plural false_negatives) false_positives
(plural false_positives)
in
match num_errors with
| 0 -> Stdlib.Ok ()
Expand Down
File renamed without changes.
File renamed without changes.
20 changes: 20 additions & 0 deletions tests/patterns/dockerfile/cmd-named-ellipsis.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
FROM debian

# MATCH:
CMD echo hello

# MATCH:
CMD ls \
| tac

# MATCH:
CMD ls \
-l

# MATCH:
CMD ls \
-l; # blah \
echo yay

# MATCH:
CMD ["ls"]
1 change: 1 addition & 0 deletions tests/patterns/dockerfile/cmd-named-ellipsis.sgrep
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CMD $...ARGS
2 changes: 2 additions & 0 deletions tests/patterns/dockerfile/entrypoint-ellipsis.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# MATCH:
ENTRYPOINT ["a"]
1 change: 1 addition & 0 deletions tests/patterns/dockerfile/entrypoint-ellipsis.sgrep
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ENTRYPOINT ...
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# MATCH:
ENTRYPOINT ["a"]
1 change: 1 addition & 0 deletions tests/patterns/dockerfile/entrypoint-named-ellipsis.sgrep
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ENTRYPOINT $...ARGS
2 changes: 1 addition & 1 deletion tests/semgrep-rules

0 comments on commit e596f43

Please sign in to comment.