fix: checks for some url based attacks

Disallowed path traversal and whitespaces in API URLs. Also added explicit checks in some `GitHubType` structs, primarily those that have string names which are interpolated into URLs. Added tests.

---

Also included these other commits, unrelated but required to fix other issues and make the CI pass:
- Bumped `actions/cache` in github workflow to v4
- Commented out two failing tests with reasons
- Fixed the topics method. It was broken in nightly because of difference in behavior of vcat when a single element vector is used. made the response uniform across julia versions.
- Made the JWT payload predictable for tests to pass

Author: nkottary

.github/workflows/ci.yml

@@ -30,7 +30,7 @@ jobs:
         with:
           version: ${{ matrix.version }}
           arch: ${{ matrix.arch }}
-      - uses: actions/cache@v1
+      - uses: actions/cache@v4
         env:
           cache-name: cache-artifacts
         with:

Project.toml

@@ -1,6 +1,6 @@
 name = "GitHub"
 uuid = "bc5e4493-9b4d-5f90-b8aa-2b2bcaad7a26"
-version = "5.9.0"
+version = "5.9.1"
 
 [deps]
 Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"

src/owners/owners.jl

@@ -33,7 +33,7 @@ end
 
 Owner(login::AbstractString, isorg = false) = Owner(Dict("login" => login, "type" => isorg ? "Organization" : "User"))
 
-namefield(owner::Owner) = owner.login
+namefield(owner::Owner) = check_disallowed_name_pattern(owner.login)
 
 typprefix(isorg) = isorg ? "orgs" : "users"
 

src/repositories/repositories.jl

@@ -38,7 +38,7 @@ end
 
 Repo(full_name::AbstractString) = Repo(Dict("full_name" => full_name))
 
-namefield(repo::Repo) = repo.full_name
+namefield(repo::Repo) = check_disallowed_name_pattern(repo.full_name)
 
 ###############
 # API Methods #
@@ -141,7 +141,7 @@ end
 
 @api_default function topics(api::GitHubAPI, repo; options...)
     results, page_data = gh_get_paged_json(api, "/repos/$(name(repo))/topics"; options...)
-    return convert(Vector{String}, results["names"]), page_data
+    return convert(Vector{String}, results[1]["names"]), page_data
 end
 
 @api_default function set_topics(api::GitHubAPI, repo, topics; options...)

src/repositories/secrets.jl

@@ -10,7 +10,7 @@ end
 
 Secret(name::AbstractString) = Secret(Dict("name" => name))
 
-namefield(secret::Secret) = secret.name
+namefield(secret::Secret) = check_disallowed_name_pattern(secret.name)
 
 ##################
 # PublicKey Type #

src/utils/auth.jl

@@ -43,15 +43,12 @@ function base64_to_base64url(string)
 end
 
 function JWTAuth(app_id::Int, key::MbedTLS.PKContext; iat = now(Dates.UTC), exp_mins = 1)
-    algo = base64_to_base64url(base64encode(JSON.json(Dict(
-        "alg" => "RS256",
-        "typ" => "JWT"
-    ))))
-    data = base64_to_base64url(base64encode(JSON.json(Dict(
-        "iat" => trunc(Int64, Dates.datetime2unix(iat)),
-        "exp" => trunc(Int64, Dates.datetime2unix(iat+Dates.Minute(exp_mins))),
-        "iss" => app_id
-    ))))
+    algo = base64_to_base64url(base64encode("{\"typ\":\"JWT\",\"alg\":\"RS256\"}"))
+
+    jwt_iat = trunc(Int64, Dates.datetime2unix(iat))
+    jwt_exp = trunc(Int64, Dates.datetime2unix(iat+Dates.Minute(exp_mins)))
+    data = base64_to_base64url(base64encode("{\"exp\":$(jwt_exp),\"iat\":$(jwt_iat),\"iss\":$(app_id)}"))
+
     signature = base64_to_base64url(base64encode(MbedTLS.sign(key, MbedTLS.MD_SHA256,
         MbedTLS.digest(MbedTLS.MD_SHA256, string(algo,'.',data)), RNG[])))
     JWTAuth(string(algo,'.',data,'.',signature))

src/utils/requests.jl

@@ -45,7 +45,13 @@ end
 # Default API URIs #
 ####################
 
-api_uri(api::GitHubWebAPI, path) = URIs.URI(api.endpoint, path = api.endpoint.path * path)
+function api_uri(api::GitHubWebAPI, path)
+    # do not allow path traversal
+    if occursin(r"(^|/)\.\.(\/|$)", path)
+        throw(ArgumentError("Invalid API path: '$path'"))
+    end
+    return URIs.URI(api.endpoint, path = api.endpoint.path * path)
+end
 api_uri(api::GitHubAPI, path) = error("URI retrieval not implemented for this API type")
 
 #######################
@@ -144,7 +150,11 @@ end
 # for APIs which return just a list
 function gh_get_paged_json(api, endpoint = ""; options...)
     results, page_data = github_paged_get(api, endpoint; options...)
-    return mapreduce(r -> JSON.parse(HTTP.payload(r, String)), vcat, results), page_data
+    parsed_results = mapreduce(r -> JSON.parse(HTTP.payload(r, String)), vcat, results)
+    if !(isa(parsed_results, Vector))
+        parsed_results = [parsed_results]
+    end
+    return parsed_results, page_data
 end
 
 # for APIs which return a Dict(key => list, "total_count" => count)
@@ -183,3 +193,21 @@ function handle_response_error(r::HTTP.Response)
                 "\tErrors: $errors"))...)
     end
 end
+
+###############
+# Validations #
+###############
+
+check_disallowed_name_pattern(v) = v
+function check_disallowed_name_pattern(str::AbstractString)
+    # do not allow path traversal in names
+    if occursin(r"\.\.", str)
+        throw(ArgumentError("name cannot contain path traversal"))
+    end
+    # do not allow new lines or carriage returns or any other whitespace in names
+    if occursin(r"\s", str)
+        throw(ArgumentError("name cannot contain line breaks"))
+    end
+
+    return str
+end

test/ghtype_tests.jl

@@ -1381,3 +1381,26 @@ end
     @test License(license_json) == license_result
     @test name(License(license_json["spdx_id"])) == name(license_result)
 end
+
+@testset "Disallowed Names" begin
+    testrepo = Repo(full_name="octocat/../julialang/julia")
+    @test_throws ArgumentError GitHub.name(testrepo)
+    testrepo = Repo(full_name="julialang/julia HTTP/1.1\r\nFoo: bar\r\nbaz:")
+    @test_throws ArgumentError GitHub.name(testrepo)
+    testrepo = Repo(full_name="julialang/julia")
+    @test GitHub.name(testrepo) == "julialang/julia"
+
+    testowner = Owner("julialang/../octocat")
+    @test_throws ArgumentError GitHub.name(testowner)
+    testowner = Owner("julialang HTTP/1.1\r\nFoo: bar\r\nbaz:")
+    @test_throws ArgumentError GitHub.name(testowner)
+    testowner = Owner("julialang")
+    @test GitHub.name(testowner) == "julialang"
+
+    testsecret = Secret("ABC/../octocat")
+    @test_throws ArgumentError GitHub.name(testsecret)
+    testsecret = Secret("ABC HTTP/1.1\r\nFoo: bar\r\nbaz:")
+    @test_throws ArgumentError GitHub.name(testsecret)
+    testsecret = Secret("ABC")
+    @test GitHub.name(testsecret) == "ABC"
+end

test/read_only_api_tests.jl

@@ -52,9 +52,10 @@ auth = authenticate(string(circshift(["bcc", "3fc", "03a", "33e",
     @test hasghobj(ghjl, first(repos(julweb; auth = auth)))
 
     # test sshkey/gpgkey retrieval
-    @test GitHub.sshkeys(testuser2; auth = auth)[1][1]["key"] == testuser2_sshkey
-    @test startswith(GitHub.gpgkeys("JuliaTagBot"; auth = auth)[1][1]["raw_key"],
-                     "-----BEGIN PGP PUBLIC KEY BLOCK-----")
+    # Test commented out because testuser2 seems to have been deleted or the key removed
+    # @test GitHub.sshkeys(testuser2; auth = auth)[1][1]["key"] == testuser2_sshkey
+    # @test startswith(GitHub.gpgkeys("JuliaTagBot"; auth = auth)[1][1]["raw_key"],
+    #                  "-----BEGIN PGP PUBLIC KEY BLOCK-----")
 
     # test membership queries
     @test GitHub.check_membership(julweb, testuser; auth = auth)
@@ -108,7 +109,8 @@ end
     # FIXME: for some reason, the GitHub API reports empty statuses on the GitHub.jl repo
     let ghjl = Repo("JuliaLang/julia"), testcommit = Commit("3200219b1f7e2681ece9e4b99bda48586fab8a93")
         @test status(ghjl, testcommit; auth = auth).sha == name(testcommit)
-        @test !(isempty(first(statuses(ghjl, testcommit; auth = auth))))
+        # The statuses API seems to be broken / not documented correctly. Ref: https://github.com/orgs/community/discussions/55455
+        # @test !(isempty(first(statuses(ghjl, testcommit; auth = auth))))
     end
 
     # test GitHub.comment, GitHub.comments
@@ -303,3 +305,9 @@ end
     topics_obj, page_data = topics("JuliaLang/julia"; auth = auth)
     @test length(topics_obj) > 0
 end
+
+@testset "Prevent Path Traversal" begin
+    @test_throws ArgumentError GitHub.api_uri(GitHub.DEFAULT_API, "/repos/foo/../bar")
+    @test_throws ArgumentError GitHub.api_uri(GitHub.DEFAULT_API, "/repos/foo/../bar")
+    @test string(GitHub.api_uri(GitHub.DEFAULT_API, "/repos/foo/bar")) == "https://api.github.com/repos/foo/bar"
+end