safer gettreesha (#449)

* safer gettreesha

The `gittreesha` method executes external commands with parameters that could be external inputs. This validates some of the inputs and changes the way `Cmd` is constructed to make it safer.
Added tests.

* rebased and bumped patch version

Author: tanmaykm

Project.toml

@@ -1,7 +1,7 @@
 name = "Registrator"
 uuid = "4418983a-e44d-11e8-3aec-9789530b3b3e"
 authors = ["Stefan Karpinski <[email protected]>"]
-version = "1.9.4"
+version = "1.9.5"
 
 [deps]
 AutoHashEquals = "15f4f7f2-30c1-5605-9d31-71845cf9641f"

src/webui/gitutils.jl

@@ -41,6 +41,16 @@ macro gf_bool(ex::Expr)
     end
 end
 
+# disallowed patterns
+# ../, ..\, /.., \.., ./, .\, /./, \.\
+const PATH_TRAVERSAL = r"(?:\.{2,}[\/\\]|\.{1,}[\/\\]|[\/\\]\.{2,}|[\/\\]\.{1,}[\/\\])"
+function is_safe_clone_url(s)
+    if occursin(PATH_TRAVERSAL, s)
+        return false
+    end
+    return true
+end
+
 # Split a repo path into its owner and name.
 function splitrepo(url::AbstractString)
     url = replace(url, r"(.*).git$" => s"\1")
@@ -242,12 +252,17 @@ function gettreesha(
 )
     return try
         url = cloneurl(r)
+        if !is_safe_clone_url(url)
+            throw(ArgumentError("Invalid or unsafe clone URL"))
+        end
         mktempdir() do dir
             dest = joinpath(dir, r.name)
             withpasswd(url) do url, env
-                run(Cmd(`git clone --bare $url $dest`; env))
+                # let Cmd interpolate strings safely
+                run(Cmd(Cmd(String["git", "clone", "--bare", string(url), dest]); env))
             end
-            readchomp(`git -C $dest rev-parse $ref:$subdir`), ""
+            # let Cmd interpolate strings safely
+            readchomp(Cmd(String["git", "-C", dest, "rev-parse", "$ref:$subdir"]))
         end
     catch ex
         @error "Exception while getting tree SHA" exception=(ex, catch_backtrace())

test/webui/gitutils.jl

@@ -13,6 +13,41 @@ end
 
 restoreconfig!()
 
+@testset "gittreesha" begin
+    org = GitHub.User(login="JuliaLang")
+
+    public_repo_of_org = GitHub.Repo(name="Example.jl", private=false, owner=org, organization=org, permissions = GitHub.Permissions(admin = true, push = false, pull = true), clone_url="https://github.com/JuliaLang/Example.jl.git")
+    example_master_treesha = Base.redirect_stderr(devnull) do
+        Registrator.WebUI.gettreesha(public_repo_of_org, "master", "")
+    end
+    @test length(example_master_treesha) == 40
+
+    ret, err = Base.redirect_stderr(devnull) do
+        Registrator.WebUI.gettreesha(public_repo_of_org, "mas ter", "")
+    end
+    @test ret === nothing
+    @test err == "Exception while getting tree SHA"
+
+    ret, err = Base.redirect_stderr(devnull) do
+        Registrator.WebUI.gettreesha(public_repo_of_org, "master", "src/../test")
+    end
+    @test ret === nothing
+    @test err == "Exception while getting tree SHA"
+
+    ret, err = Base.redirect_stderr(devnull) do
+        Registrator.WebUI.gettreesha(public_repo_of_org, "master", "src test")
+    end
+    @test ret === nothing
+    @test err == "Exception while getting tree SHA"
+
+    unsafe_repo = GitHub.Repo(name="Example.jl", private=false, owner=org, organization=org, permissions = GitHub.Permissions(admin = true, push = false, pull = true), clone_url="https://github.com/JuliaLang/../unsafe/Example.jl.git")
+    ret, err = Base.redirect_stderr(devnull) do
+        Registrator.WebUI.gettreesha(unsafe_repo, "master", "")
+    end
+    @test ret === nothing
+    @test err == "Exception while getting tree SHA"
+end
+
 @testset "isauthorized" begin
     @test isauthorized("username", "reponame") == AuthFailure("Unkown user type or repo type")
     mock_provider!()