Skip to content

Commit

Permalink
Prevent directory traversal (along piecemeal way, with minor fix.)
Browse files Browse the repository at this point in the history
See <f656edfe35651b54eec50d814e79d079f8eea7c4>.

Sanitizes «...git» in addition to other sanitization.

Adds unit and integration test.
  • Loading branch information
jdhealy committed Jun 17, 2020
1 parent f656edf commit 249aabd
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
21 changes: 21 additions & 0 deletions IntegrationTests/relink-conflicting-not-checked-in-symlink.bats
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,24 @@ carthage-and-check-project-symlink() {
carthage-and-check-project-symlink update --no-build --no-use-binaries

}

@test "with conflicting not-checked-in symlink in «Carthage/Checkouts» of dependency pathologically named «...git», carthage «bootstrap, update, update» should — sanitizing throughout — unlink, then write symlink there" {

mv "${extracted_directory:?}/SourceRepos/TestFramework1" "${extracted_directory}/SourceRepos/...git"

rm Cartfile

cat > Cartfile <<-EOF
git "file://${extracted_directory}/SourceRepos/...git" "relink-conflicting-not-checked-in-syminks"
EOF

carthage bootstrap --no-build --no-use-binaries
check-symlink "$(project_directory)/Carthage/Checkouts/../Carthage/Checkouts/TestFramework2"
# carthage should have sanitized the former «TestFramework1» from «...git» to non–path-traversing «..»…

carthage update --no-build --no-use-binaries
check-symlink "$(project_directory)/Carthage/Checkouts/../Carthage/Checkouts/TestFramework2"

carthage update --no-build --no-use-binaries
check-symlink "$(project_directory)/Carthage/Checkouts/../Carthage/Checkouts/TestFramework2"
}
5 changes: 2 additions & 3 deletions Source/CarthageKit/GitURL.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ public struct GitURL {
.split(omittingEmptySubsequences: true) { $0 == "/" }
.last
.map(String.init)

precondition(lastComponent != .some("")) /* because of `omittingEmptySubsequences` */
.map(strippingGitSuffix)

/// Potentially used to prevent backwards or noop directory traversal via «FULL STOP» characters…
/// …by deploying the «FULLWIDTH FULL STOP» character.
Expand All @@ -67,7 +66,7 @@ public struct GitURL {
return String(replacementForEntirelyCharactersOfFullStop)
}

return lastComponent.map(strippingGitSuffix)
return lastComponent
}

public init(_ urlString: String) {
Expand Down
6 changes: 6 additions & 0 deletions Tests/CarthageKitTests/DependencySpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,12 @@ class DependencySpec: QuickSpec {
expect(dependency.name) == "\u{FF0E}\u{FF0E}"
}

it("should sanitize if the given URL string is (pathologically) «...git»") {
let dependency = Dependency.git(GitURL("...git"))

expect(dependency.name) == "\u{FF0E}\u{FF0E}"
}

it ("should be the directory name if the given URL string is (pathologically) prefixed by «../» with (pathologically) no URL scheme") {
let dependency = Dependency.git(GitURL("../myproject"))

Expand Down

0 comments on commit 249aabd

Please sign in to comment.