Escape paths and args in shell commands

Basic escaping is done only. Fancy hex sequences are not covered, but
this should otherwise take care of the most common issues.

Fixes #497

4 files changed, 72 insertions, 39 deletions

diff --git a/src/rebar_file_utils.erl b/src/rebar_file_utils.erl
index b4cdc27..da58c00 100644
--- a/src/rebar_file_utils.erl
+++ b/src/rebar_file_utils.erl
@@ -98,7 +98,8 @@ symlink_or_copy(Source, Target) ->
 win32_symlink(Source, Target) ->
     Res = rebar_utils:sh(
             ?FMT("cmd /c mklink /j \"~s\" \"~s\"",
-                 [filename:nativename(Target), filename:nativename(Source)]),
+                 [rebar_utils:escape_double_quotes(filename:nativename(Target)),
+                  rebar_utils:escape_double_quotes(filename:nativename(Source))]),
             [{use_stdout, false}, return_on_error]),
     case win32_ok(Res) of
         true -> ok;
@@ -115,7 +116,7 @@ win32_symlink(Source, Target) ->
 rm_rf(Target) ->
     case os:type() of
         {unix, _} ->
-            EscTarget = escape_path(Target),
+            EscTarget = rebar_utils:escape_chars(Target),
             {ok, []} = rebar_utils:sh(?FMT("rm -rf ~s", [EscTarget]),
                                       [{use_stdout, false}, abort_on_error]),
             ok;
@@ -134,10 +135,10 @@ cp_r([], _Dest) ->
 cp_r(Sources, Dest) ->
     case os:type() of
         {unix, _} ->
-            EscSources = [escape_path(Src) || Src <- Sources],
+            EscSources = [rebar_utils:escape_chars(Src) || Src <- Sources],
             SourceStr = string:join(EscSources, " "),
             {ok, []} = rebar_utils:sh(?FMT("cp -R ~s \"~s\"",
-                                           [SourceStr, Dest]),
+                                           [SourceStr, rebar_utils:escape_double_quotes(Dest)]),
                                       [{use_stdout, false}, abort_on_error]),
             ok;
         {win32, _} ->
@@ -149,8 +150,8 @@ cp_r(Sources, Dest) ->
 mv(Source, Dest) ->
     case os:type() of
         {unix, _} ->
-            EscSource = escape_path(Source),
-            EscDest = escape_path(Dest),
+            EscSource = rebar_utils:escape_chars(Source),
+            EscDest = rebar_utils:escape_chars(Dest),
             {ok, []} = rebar_utils:sh(?FMT("mv ~s ~s", [EscSource, EscDest]),
                                       [{use_stdout, false}, abort_on_error]),
             ok;
@@ -158,13 +159,13 @@ mv(Source, Dest) ->
             Cmd = case filelib:is_dir(Source) of
                       true ->
                           ?FMT("robocopy /move /s \"~s\" \"~s\" 1> nul",
-                               [filename:nativename(Source),
-                                filename:nativename(Dest)]);
+                               [rebar_utils:escape_double_quotes(filename:nativename(Source)),
+                                rebar_utils:escape_double_quotes(filename:nativename(Dest))]);
                       false ->
                           ?FMT("robocopy /move /s \"~s\" \"~s\" \"~s\" 1> nul",
-                               [filename:nativename(filename:dirname(Source)),
-                                filename:nativename(Dest),
-                                filename:basename(Source)])
+                               [rebar_utils:escape_double_quotes(filename:nativename(filename:dirname(Source))),
+                                rebar_utils:escape_double_quotes(filename:nativename(Dest)),
+                                rebar_utils:escape_double_quotes(filename:basename(Source))])
                   end,
             Res = rebar_utils:sh(Cmd,
                         [{use_stdout, false}, return_on_error]),
@@ -250,7 +251,7 @@ touch(Path) ->
 delete_each_dir_win32([]) -> ok;
 delete_each_dir_win32([Dir | Rest]) ->
     {ok, []} = rebar_utils:sh(?FMT("rd /q /s \"~s\"",
-                                   [filename:nativename(Dir)]),
+                                   [rebar_utils:escape_double_quotes(filename:nativename(Dir))]),
                               [{use_stdout, false}, return_on_error]),
     delete_each_dir_win32(Rest).
 
@@ -260,13 +261,13 @@ xcopy_win32(Source,Dest)->
     Cmd = case filelib:is_dir(Source) of
               true ->
                   ?FMT("robocopy \"~s\" \"~s\" /e /is 1> nul",
-                       [filename:nativename(Source),
-                        filename:nativename(Dest)]);
+                       [rebar_utils:escape_double_quotes(filename:nativename(Source)),
+                        rebar_utils:escape_double_quotes(filename:nativename(Dest))]);
               false ->
                   ?FMT("robocopy \"~s\" \"~s\" \"~s\" /e /is 1> nul",
-                       [filename:nativename(filename:dirname(Source)),
-                        filename:nativename(Dest),
-                        filename:basename(Source)])
+                       [rebar_utils:escape_double_quotes(filename:nativename(filename:dirname(Source))),
+                        rebar_utils:escape_double_quotes(filename:nativename(Dest)),
+                        rebar_utils:escape_double_quotes(filename:basename(Source))])
           end,
     Res = rebar_utils:sh(Cmd,
                 [{use_stdout, false}, return_on_error]),
@@ -318,5 +319,3 @@ cp_r_win32(Source,Dest) ->
                   end, filelib:wildcard(Source)),
     ok.
 
-escape_path(Str) ->
-    re:replace(Str, "([ ()?])", "\\\\&", [global, {return, list}]).
diff --git a/src/rebar_git_resource.erl b/src/rebar_git_resource.erl
index a0690bf..aec1535 100644
--- a/src/rebar_git_resource.erl
+++ b/src/rebar_git_resource.erl
@@ -16,7 +16,7 @@ lock(AppDir, {git, Url, _}) ->
 lock(AppDir, {git, Url}) ->
     AbortMsg = io_lib:format("Locking of git dependency failed in ~s", [AppDir]),
     {ok, VsnString} =
-        rebar_utils:sh("git --git-dir=\"" ++ AppDir ++ "/.git\" rev-parse --verify HEAD",
+        rebar_utils:sh("git --git-dir=\"" ++ rebar_utils:escape_double_quotes(AppDir) ++ "/.git\" rev-parse --verify HEAD",
                        [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
     Ref = string:strip(VsnString, both, $\n),
     {git, Url, {ref, Ref}}.
@@ -32,10 +32,11 @@ needs_update(Dir, {git, Url, {tag, Tag}}) ->
     not ((Current1 =:= Tag) andalso compare_url(Dir, Url));
 needs_update(Dir, {git, Url, {branch, Branch}}) ->
     %% Fetch remote so we can check if the branch has changed
-    {ok, _} = rebar_utils:sh(?FMT("git fetch origin ~s", [Branch]),
+    SafeBranch = rebar_utils:escape_chars(Branch),
+    {ok, _} = rebar_utils:sh(?FMT("git fetch origin ~s", [SafeBranch]),
                              [{cd, Dir}]),
     %% Check for new commits to origin/Branch
-    {ok, Current} = rebar_utils:sh(?FMT("git log HEAD..origin/~s --oneline", [Branch]),
+    {ok, Current} = rebar_utils:sh(?FMT("git log HEAD..origin/~s --oneline", [SafeBranch]),
                                    [{cd, Dir}]),
     ?DEBUG("Checking git branch ~s for updates", [Branch]),
     not ((Current =:= []) andalso compare_url(Dir, Url));
@@ -93,24 +94,33 @@ download(Dir, {git, Url, ""}, State) ->
 download(Dir, {git, Url, {branch, Branch}}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("git clone ~s ~s -b ~s --single-branch",
-                       [Url, filename:basename(Dir), Branch]),
+                       [rebar_utils:escape_chars(Url),
+                        rebar_utils:escape_chars(filename:basename(Dir)),
+                        rebar_utils:escape_chars(Branch)]),
                    [{cd, filename:dirname(Dir)}]);
 download(Dir, {git, Url, {tag, Tag}}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("git clone ~s ~s -b ~s --single-branch",
-                        [Url, filename:basename(Dir), Tag]),
+                       [rebar_utils:escape_chars(Url),
+                        rebar_utils:escape_chars(filename:basename(Dir)),
+                        rebar_utils:escape_chars(Tag)]),
                    [{cd, filename:dirname(Dir)}]);
 download(Dir, {git, Url, {ref, Ref}}, _State) ->
     ok = filelib:ensure_dir(Dir),
-    rebar_utils:sh(?FMT("git clone -n ~s ~s", [Url, filename:basename(Dir)]),
+    rebar_utils:sh(?FMT("git clone -n ~s ~s",
+                        [rebar_utils:escape_chars(Url),
+                         rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]),
     rebar_utils:sh(?FMT("git checkout -q ~s", [Ref]), [{cd, Dir}]);
 download(Dir, {git, Url, Rev}, _State) ->
     ?WARN("WARNING: It is recommended to use {branch, Name}, {tag, Tag} or {ref, Ref}, otherwise updating the dep may not work as expected.", []),
     ok = filelib:ensure_dir(Dir),
-    rebar_utils:sh(?FMT("git clone -n ~s ~s", [Url, filename:basename(Dir)]),
+    rebar_utils:sh(?FMT("git clone -n ~s ~s",
+                        [rebar_utils:escape_chars(Url),
+                         rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]),
-    rebar_utils:sh(?FMT("git checkout -q ~s", [Rev]), [{cd, Dir}]).
+    rebar_utils:sh(?FMT("git checkout -q ~s", [rebar_utils:escape_chars(Rev)]),
+                   [{cd, Dir}]).
 
 make_vsn(Dir) ->
     {Vsn, RawRef, RawCount} = collect_default_refcount(Dir),
@@ -162,7 +172,7 @@ get_patch_count(Dir, RawRef) ->
     AbortMsg = "Getting rev-list of git dep failed in " ++ Dir,
     Ref = re:replace(RawRef, "\\s", "", [global]),
     Cmd = io_lib:format("git rev-list ~s..HEAD",
-                         [Ref]),
+                        [rebar_utils:escape_chars(Ref)]),
     {ok, PatchLines} = rebar_utils:sh(Cmd,
                                         [{use_stdout, false},
                                          {cd, Dir},
diff --git a/src/rebar_hg_resource.erl b/src/rebar_hg_resource.erl
index a67abb9..7d03eda 100644
--- a/src/rebar_hg_resource.erl
+++ b/src/rebar_hg_resource.erl
@@ -57,26 +57,34 @@ download(Dir, {hg, Url, ""}, State) ->
 download(Dir, {hg, Url, {branch, Branch}}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("hg clone -q -b ~s ~s ~s",
-                       [Branch, Url, filename:basename(Dir)]),
+                       [rebar_utils:escape_chars(Branch),
+                        rebar_utils:escape_chars(Url),
+                        rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]);
 download(Dir, {hg, Url, {tag, Tag}}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("hg clone -q -u ~s ~s ~s",
-                        [Tag, Url, filename:basename(Dir)]),
+                        [rebar_utils:escape_chars(Tag),
+                         rebar_utils:escape_chars(Url),
+                         rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]);
 download(Dir, {hg, Url, {ref, Ref}}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("hg clone -q -r ~s ~s ~s",
-                        [Ref, Url, filename:basename(Dir)]),
+                        [rebar_utils:escape_chars(Ref),
+                         rebar_utils:escape_chars(Url),
+                         rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]);
 download(Dir, {hg, Url, Rev}, _State) ->
     ok = filelib:ensure_dir(Dir),
     rebar_utils:sh(?FMT("hg clone -q -r ~s ~s ~s",
-                        [Rev, Url, filename:basename(Dir)]),
+                        [rebar_utils:escape_chars(Rev),
+                         rebar_utils:escape_chars(Url),
+                         rebar_utils:escape_chars(filename:basename(Dir))]),
                    [{cd, filename:dirname(Dir)}]).
 
 make_vsn(Dir) ->
-    BaseHg = "hg -R '" ++ Dir ++ "' ",
+    BaseHg = "hg -R \"" ++ rebar_utils:escape_double_quotes(Dir) ++ "\" ",
     Ref = get_ref(Dir),
     Cmd = BaseHg ++ "log --template \"{latesttag}+build.{latesttagdistance}.rev.{node|short}\""
           " --rev " ++ Ref,
@@ -95,23 +103,23 @@ make_vsn(Dir) ->
 %%% Internal functions
 
 compare_url(Dir, Url) ->
-    CurrentUrl = string:strip(os:cmd("hg -R '" ++ Dir ++"' paths default"), both, $\n),
+    CurrentUrl = string:strip(os:cmd("hg -R \"" ++ rebar_utils:escape_double_quotes(Dir) ++"\" paths default"), both, $\n),
     CurrentUrl1 = string:strip(CurrentUrl, both, $\r),
     parse_hg_url(CurrentUrl1) =:= parse_hg_url(Url).
 
 get_ref(Dir) ->
     AbortMsg = io_lib:format("Get ref of hg dependency failed in ~s", [Dir]),
     {ok, RefString} =
-        rebar_utils:sh("hg -R '" ++ Dir ++ "' --debug id -i",
+        rebar_utils:sh("hg -R \"" ++ rebar_utils:escape_double_quotes(Dir) ++ "\" --debug id -i",
                        [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
     string:strip(RefString, both, $\n).
 
 get_tag_distance(Dir, Ref) ->
     AbortMsg = io_lib:format("Get tag distance of hg dependency failed in ~s", [Dir]),
     {ok, LogString} =
-        rebar_utils:sh("hg -R '" ++ Dir ++ "' "
+        rebar_utils:sh("hg -R \"" ++ rebar_utils:escape_double_quotes(Dir) ++ "\" "
                       "log --template \"{latesttag}-{latesttagdistance}\n\" "
-                      "--rev " ++ Ref,
+                      "--rev " ++ rebar_utils:escape_chars(Ref),
                       [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
     Log = string:strip(LogString,
                        both, $\n),
@@ -121,7 +129,8 @@ get_tag_distance(Dir, Ref) ->
 get_branch_ref(Dir, Branch) ->
     AbortMsg = io_lib:format("Get branch ref of hg dependency failed in ~s", [Dir]),
     {ok, BranchRefString} =
-        rebar_utils:sh("hg -R '" ++ Dir ++ "' log --template \"{node}\n\" --rev " ++ Branch,
+        rebar_utils:sh("hg -R \"" ++ rebar_utils:escape_double_quotes(Dir) ++
+                       "\" log --template \"{node}\n\" --rev " ++ rebar_utils:escape_chars(Branch),
                        [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
     string:strip(BranchRefString, both, $\n).
 
diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl
index ebdf0fe..6eb4f4b 100644
--- a/src/rebar_utils.erl
+++ b/src/rebar_utils.erl
@@ -57,7 +57,10 @@
          tup_umerge/2,
          tup_sort/1,
          line_count/1,
-         set_httpc_options/0]).
+         set_httpc_options/0,
+         escape_chars/1,
+         escape_double_quotes/1,
+         escape_double_quotes_weak/1]).
 
 %% for internal use only
 -export([otp_release/0]).
@@ -684,3 +687,15 @@ set_httpc_options(_, []) ->
 set_httpc_options(Scheme, Proxy) ->
     {ok, {_, _, Host, Port, _, _}} = http_uri:parse(Proxy),
     httpc:set_options([{Scheme, {{Host, Port}, []}}], rebar).
+
+%% escape\ as\ a\ shell\?
+escape_chars(Str) ->
+    re:replace(Str, "([ ()?`!$])", "\\\\&", [global, {return, list}]).
+
+%% "escape inside these"
+escape_double_quotes(Str) ->
+    re:replace(Str, "([\"\\\\`!$*])", "\\\\&", [global, {return, list}]).
+
+%% "escape inside these" but allow *
+escape_double_quotes_weak(Str) ->
+    re:replace(Str, "([\"\\\\`!$])", "\\\\&", [global, {return, list}]).