Skip to content

Commit 4e48928

Browse files
sunny-sesunny-se
committed
Merge remote-tracking branches 'origin/fix/DEVA11Y-473-https-download-url', 'origin/fix/DEVA11Y-474-https-shell-download', 'origin/fix/DEVA11Y-475-remove-cli-self-update', 'origin/fix/DEVA11Y-476-pin-semgrep-image', 'origin/fix/DEVA11Y-477-pin-spm-dependency', 'origin/fix/DEVA11Y-478-remove-spm-self-update', 'origin/fix/DEVA11Y-479-block-file-scheme', 'origin/fix/DEVA11Y-480-sanitize-version-string', 'origin/fix/DEVA11Y-481-restrict-network-scope', 'origin/fix/DEVA11Y-482-atomic-cache-update', 'origin/fix/DEVA11Y-483-spm-tmpdir-isolation' and 'origin/fix/DEVA11Y-484-extraction-size-limit' into security/all-fixes-combined
12 parents 1483c93 + ac22720 + ecd1c1a + 1f0dc12 + bd124c0 + 0cfc181 + 7e92021 + 95f7990 + 54872ac + f00a0a3 + 2d72ba0 + 05c39bf commit 4e48928

9 files changed

Lines changed: 108 additions & 110 deletions

File tree

‎.github/workflows/Semgrep.yml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ jobs:
2626
runs-on: ubuntu-latest
2727

2828
container:
29-
# A Docker image with Semgrep installed. Do not change this.
30-
image: returntocorp/semgrep
29+
# Pinned by digest for supply-chain integrity (DEVA11Y-476).
30+
# To update: docker manifest inspect returntocorp/semgrep:latest
31+
image: returntocorp/semgrep@sha256:f682953ce85e3725f4a4dd94bd7ad13e570bb6b2c7a8cf7c6e38a9eac89239b2
3132

3233
# Skip any PR created by dependabot to avoid permission issues:
3334
if: (github.actor != 'dependabot[bot]')

‎Package.swift‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,7 @@ let package = Package(
1919
),
2020
permissions: [
2121
.allowNetworkConnections(
22-
// scope: .all(ports: []),
23-
scope: .all(),
22+
scope: .all(ports: []),
2423
reason: "Please allow network connection permission to authenticate and run accessibility rules."
2524
),
2625
.writeToPackageDirectory(reason: "Please allow writing to package directory for logging.")

‎Plugins/BrowserStackAccessibilityLint/BrowserStackAccessibilityLint.swift‎

Lines changed: 62 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,13 @@ private func parseOverride(urlString: String?) throws -> URL? {
100100
guard let urlString = urlString, !urlString.isEmpty else {
101101
return nil
102102
}
103-
if let url = URL(string: urlString), let scheme = url.scheme, ["http", "https", "file"].contains(scheme.lowercased()) {
104-
return url
103+
guard let url = URL(string: urlString), let scheme = url.scheme else {
104+
throw PluginError("Invalid download URL: \(urlString). Only HTTPS URLs are supported.")
105+
}
106+
guard scheme.lowercased() == "https" else {
107+
throw PluginError("Unsupported URL scheme '\(scheme)' in download URL. Only HTTPS is allowed.")
105108
}
106-
return URL(fileURLWithPath: urlString)
109+
return url
107110
}
108111

109112
private func sanitizeArguments(_ arguments: [String]) -> [String] {
@@ -199,33 +202,40 @@ private struct BrowserStackCLIDownloader {
199202
return BrowserStackCLIArtifact(version: info.version, executableURL: expectedExecutableURL)
200203
}
201204

202-
if fileManager.fileExists(atPath: versionDirectory.path) {
203-
try fileManager.removeItem(at: versionDirectory)
204-
}
205-
try fileManager.createDirectory(at: versionDirectory, withIntermediateDirectories: true)
206-
207205
Diagnostics.remark("BrowserStackAccessibilityLint: Downloading CLI \(info.version)...")
208206

207+
// Download into a temporary directory to avoid TOCTOU races
208+
let tempDirectory = cacheRoot.appendingPathComponent(".download-\(UUID().uuidString)", isDirectory: true)
209+
try fileManager.createDirectory(at: tempDirectory, withIntermediateDirectories: true)
210+
defer { try? fileManager.removeItem(at: tempDirectory) }
211+
209212
#if os(Windows)
210-
let archiveURL = versionDirectory.appendingPathComponent("browserstack-cli.zip")
213+
let archiveURL = tempDirectory.appendingPathComponent("browserstack-cli.zip")
211214
try await download(from: info.resolvedURL, to: archiveURL)
212215
Diagnostics.remark("BrowserStackAccessibilityLint: Extracting CLI \(info.version)...")
213-
try unzip(archive: archiveURL, into: versionDirectory)
216+
try unzip(archive: archiveURL, into: tempDirectory)
214217
try? fileManager.removeItem(at: archiveURL)
215218
#else
216-
try extractWithBsdtar(from: info.resolvedURL, into: versionDirectory)
219+
try extractWithBsdtar(from: info.resolvedURL, into: tempDirectory)
217220
#endif
218221

219-
let locatedBinary = try locateExecutable(in: versionDirectory, preferredName: executableName)
220-
let finalBinaryURL: URL
221-
if locatedBinary.lastPathComponent == executableName {
222-
finalBinaryURL = locatedBinary
223-
} else {
224-
finalBinaryURL = expectedExecutableURL
225-
if fileManager.fileExists(atPath: finalBinaryURL.path) {
226-
try fileManager.removeItem(at: finalBinaryURL)
222+
let locatedBinary = try locateExecutable(in: tempDirectory, preferredName: executableName)
223+
224+
// Atomically swap: remove old version dir, move temp into place
225+
if fileManager.fileExists(atPath: versionDirectory.path) {
226+
try fileManager.removeItem(at: versionDirectory)
227+
}
228+
try fileManager.moveItem(at: tempDirectory, to: versionDirectory)
229+
230+
let finalBinaryURL = versionDirectory.appendingPathComponent(locatedBinary.lastPathComponent, isDirectory: false)
231+
if locatedBinary.lastPathComponent != executableName {
232+
let expectedURL = versionDirectory.appendingPathComponent(executableName, isDirectory: false)
233+
if fileManager.fileExists(atPath: expectedURL.path) {
234+
try fileManager.removeItem(at: expectedURL)
227235
}
228-
try fileManager.moveItem(at: locatedBinary, to: finalBinaryURL)
236+
try fileManager.moveItem(at: finalBinaryURL, to: expectedURL)
237+
try ensureExecutablePermissions(at: expectedURL)
238+
return BrowserStackCLIArtifact(version: info.version, executableURL: expectedURL)
229239
}
230240

231241
try ensureExecutablePermissions(at: finalBinaryURL)
@@ -285,6 +295,8 @@ private struct BrowserStackCLIDownloader {
285295
let message = String(data: tarError.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
286296
forwardExit(code: bsdtar.terminationStatus, message: message.isEmpty ? "bsdtar failed to extract BrowserStack CLI." : message)
287297
}
298+
299+
try validateExtractedSize(of: directory)
288300
}
289301

290302
private func extractLocalArchive(at archiveURL: URL, into directory: URL) throws {
@@ -301,6 +313,11 @@ private struct BrowserStackCLIDownloader {
301313
throw PluginError("Failed to launch bsdtar: \(error.localizedDescription)")
302314
}
303315

316+
if process.terminationReason == .exit && process.terminationStatus == 0 {
317+
try validateExtractedSize(of: directory)
318+
return
319+
}
320+
304321
if process.terminationReason != .exit || process.terminationStatus != 0 {
305322
// Fall back to copying the file directly if it's already an executable.
306323
let message = String(data: errorPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
@@ -315,6 +332,24 @@ private struct BrowserStackCLIDownloader {
315332
}
316333
}
317334
}
335+
336+
private func validateExtractedSize(of directory: URL, maxBytes: UInt64 = 100 * 1024 * 1024) throws {
337+
var totalSize: UInt64 = 0
338+
let enumerator = fileManager.enumerator(
339+
at: directory,
340+
includingPropertiesForKeys: [.fileSizeKey],
341+
options: [.skipsHiddenFiles]
342+
)
343+
while let fileURL = enumerator?.nextObject() as? URL {
344+
if let size = try? fileURL.resourceValues(forKeys: [.fileSizeKey]).fileSize {
345+
totalSize += UInt64(size)
346+
if totalSize > maxBytes {
347+
try? fileManager.removeItem(at: directory)
348+
throw PluginError("Extracted archive exceeds maximum allowed size (\(maxBytes / (1024 * 1024)) MB). Possible decompression bomb.")
349+
}
350+
}
351+
}
352+
}
318353
#endif
319354

320355
private func resolveOverrideArtifact(from url: URL) async throws -> ArtifactInfo {
@@ -557,8 +592,13 @@ private func hardwareIdentifier() throws -> String {
557592
private func extractVersion(from url: URL) -> String? {
558593
let filename = url.deletingPathExtension().lastPathComponent
559594
if let range = filename.range(of: "-", options: .backwards) {
560-
let version = filename[range.upperBound...]
561-
return version.isEmpty ? nil : String(version)
595+
let version = String(filename[range.upperBound...])
596+
if version.isEmpty { return nil }
597+
// Reject path traversal and non-semver characters
598+
let allowed = CharacterSet.alphanumerics.union(CharacterSet(charactersIn: ".-+"))
599+
guard version.unicodeScalars.allSatisfy({ allowed.contains($0) }) else { return nil }
600+
guard !version.contains("..") else { return nil }
601+
return version
562602
}
563603
return nil
564604
}

‎scripts/bash/cli.sh‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,21 +78,11 @@ a11y_scan() {
7878
$BINARY_PATH a11y $EXTRA_ARGS
7979
}
8080

81-
script_self_update() {
82-
local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/bash/cli.sh"
83-
84-
updated_script=$(curl -R -z "$SCRIPT_PATH" "$remote_url")
85-
if [[ $updated_script =~ ^#! ]]; then
86-
echo "$updated_script" > "$SCRIPT_PATH"
87-
fi
88-
}
89-
9081
download_binary() {
91-
curl -R -z "$BINARY_ZIP_PATH" -L "http://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
82+
curl -R -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
9283
bsdtar -xvf "$BINARY_ZIP_PATH" -O > "$BINARY_PATH" && chmod 0775 "$BINARY_PATH"
9384
}
9485

95-
script_self_update
9686
if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then
9787
register_git_hook
9888
exit 0

‎scripts/bash/spm.sh‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#!/usr/bin/env bash -il
22

3-
[ -f "${PWD}/Package.swift" ]
4-
PACKAGE_EXISTS="$?"
3+
ORIGINAL_DIR="${PWD}"
4+
HAS_EXISTING_PACKAGE=0
5+
[ -f "${PWD}/Package.swift" ] && HAS_EXISTING_PACKAGE=1
56
GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
67
SCRIPT_PATH=$(realpath --relative-to="$GIT_ROOT" "$0" 2>/dev/null || realpath "$0")
78
SUBCOMMAND="$1"
@@ -41,26 +42,30 @@ EOF
4142
a11y_scan() {
4243
# Ensure Package.swift is removed on exit (acts like a finally block)
4344
cleanup() {
44-
if [ $PACKAGE_EXISTS -eq 0 ]; then
45+
if [ $HAS_EXISTING_PACKAGE -eq 1 ]; then
4546
return
4647
fi
47-
rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
48+
if [ -n "$WORK_DIR" ] && [ -d "$WORK_DIR" ]; then
49+
rm -rf -- "$WORK_DIR"
50+
fi
4851
}
4952
trap cleanup EXIT
5053

5154
setup() {
52-
if [ $PACKAGE_EXISTS -eq 0 ]; then
55+
if [ $HAS_EXISTING_PACKAGE -eq 1 ]; then
56+
WORK_DIR="$ORIGINAL_DIR"
5357
return
5458
fi
5559

56-
cat > Package.swift <<EOF
60+
WORK_DIR=$(mktemp -d)
61+
cat > "$WORK_DIR/Package.swift" <<EOF
5762
// swift-tools-version: 5.9
5863
import PackageDescription
5964
6065
let package = Package(
6166
name: "Dummy",
6267
dependencies: [
63-
.package(url: "https://github.com/browserstack/AccessibilityDevTools.git", branch: "main")
68+
.package(url: "https://github.com/browserstack/AccessibilityDevTools.git", revision: "0428b322b00494b19e44c20c37502a0ee31af642")
6469
],
6570
targets: []
6671
)
@@ -71,6 +76,7 @@ EOF
7176
if [[ -z "$EXTRA_ARGS" ]]; then
7277
EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
7378
fi
79+
cd "$WORK_DIR"
7480
env -i HOME="$HOME" \
7581
XCODE_VERSION_ACTUAL="$XCODE_VERSION_ACTUAL"\
7682
BROWSERSTACK_USERNAME="$BROWSERSTACK_USERNAME"\
@@ -83,16 +89,6 @@ EOF
8389
scan $EXTRA_ARGS
8490
}
8591

86-
script_self_update() {
87-
local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/bash/spm.sh"
88-
89-
updated_script=$(curl -R -z "$SCRIPT_PATH" "$remote_url")
90-
if [[ $updated_script =~ ^#! ]]; then
91-
echo "$updated_script" > "$SCRIPT_PATH"
92-
fi
93-
}
94-
95-
script_self_update
9692
if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then
9793
register_git_hook
9894
exit 0

‎scripts/fish/cli.sh‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,21 +90,11 @@ a11y_scan() {
9090
$BINARY_PATH a11y $EXTRA_ARGS
9191
}
9292

93-
script_self_update() {
94-
local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/fish/cli.sh"
95-
96-
updated_script=$(curl -R -z "$SCRIPT_PATH" "$remote_url")
97-
if [[ $updated_script =~ ^#! ]]; then
98-
echo "$updated_script" > "$SCRIPT_PATH"
99-
fi
100-
}
101-
10293
download_binary() {
103-
curl -R -z "$BINARY_ZIP_PATH" -L "http://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
94+
curl -R -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
10495
bsdtar -xvf "$BINARY_ZIP_PATH" -O > "$BINARY_PATH" && chmod 0775 "$BINARY_PATH"
10596
}
10697

107-
script_self_update
10898
if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then
10999
register_git_hook
110100
exit 0

‎scripts/fish/spm.sh‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ export BROWSERSTACK_USERNAME=$($fish_bin -lic 'echo $BROWSERSTACK_USERNAME' | ta
1313
export BROWSERSTACK_ACCESS_KEY=$($fish_bin -lic 'echo $BROWSERSTACK_ACCESS_KEY' | tail -n 1)
1414

1515
# Don't change anything after this, same as the bash equivalent
16-
[ -f "${PWD}/Package.swift" ]
17-
PACKAGE_EXISTS="$?"
16+
ORIGINAL_DIR="${PWD}"
17+
HAS_EXISTING_PACKAGE=0
18+
[ -f "${PWD}/Package.swift" ] && HAS_EXISTING_PACKAGE=1
1819
GIT_ROOT=$(git rev-parse --show-toplevel 2>/dev/null)
1920
SCRIPT_PATH=$(realpath --relative-to="$GIT_ROOT" "$0" 2>/dev/null || realpath "$0")
2021
SUBCOMMAND="$1"
@@ -54,26 +55,30 @@ EOF
5455
a11y_scan() {
5556
# Ensure Package.swift is removed on exit (acts like a finally block)
5657
cleanup() {
57-
if [ $PACKAGE_EXISTS -eq 0 ]; then
58+
if [ $HAS_EXISTING_PACKAGE -eq 1 ]; then
5859
return
5960
fi
60-
rm -f -- "${PWD}/Package.swift" "${PWD}/Package.resolved"
61+
if [ -n "$WORK_DIR" ] && [ -d "$WORK_DIR" ]; then
62+
rm -rf -- "$WORK_DIR"
63+
fi
6164
}
6265
trap cleanup EXIT
6366

6467
setup() {
65-
if [ $PACKAGE_EXISTS -eq 0 ]; then
68+
if [ $HAS_EXISTING_PACKAGE -eq 1 ]; then
69+
WORK_DIR="$ORIGINAL_DIR"
6670
return
6771
fi
6872

69-
cat > Package.swift <<EOF
73+
WORK_DIR=$(mktemp -d)
74+
cat > "$WORK_DIR/Package.swift" <<EOF
7075
// swift-tools-version: 5.9
7176
import PackageDescription
7277
7378
let package = Package(
7479
name: "Dummy",
7580
dependencies: [
76-
.package(url: "https://github.com/browserstack/AccessibilityDevTools.git", branch: "main")
81+
.package(url: "https://github.com/browserstack/AccessibilityDevTools.git", revision: "0428b322b00494b19e44c20c37502a0ee31af642")
7782
],
7883
targets: []
7984
)
@@ -84,6 +89,7 @@ EOF
8489
if [[ -z "$EXTRA_ARGS" ]]; then
8590
EXTRA_ARGS="--include **/*.swift --include **/*.xib --include **/*.storyboard"
8691
fi
92+
cd "$WORK_DIR"
8793
env -i HOME="$HOME" \
8894
XCODE_VERSION_ACTUAL="$XCODE_VERSION_ACTUAL"\
8995
BROWSERSTACK_USERNAME="$BROWSERSTACK_USERNAME"\
@@ -96,16 +102,6 @@ EOF
96102
scan $EXTRA_ARGS
97103
}
98104

99-
script_self_update() {
100-
local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/fish/spm.sh"
101-
102-
updated_script=$(curl -R -z "$SCRIPT_PATH" "$remote_url")
103-
if [[ $updated_script =~ ^#! ]]; then
104-
echo "$updated_script" > "$SCRIPT_PATH"
105-
fi
106-
}
107-
108-
script_self_update
109105
if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then
110106
register_git_hook
111107
exit 0

‎scripts/zsh/cli.sh‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,21 +89,11 @@ a11y_scan() {
8989
$BINARY_PATH a11y $EXTRA_ARGS
9090
}
9191

92-
script_self_update() {
93-
local remote_url="https://raw.githubusercontent.com/browserstack/AccessibilityDevTools/refs/heads/main/scripts/zsh/cli.sh"
94-
95-
updated_script=$(curl -R -z "$SCRIPT_PATH" "$remote_url")
96-
if [[ $updated_script =~ ^#! ]]; then
97-
echo "$updated_script" > "$SCRIPT_PATH"
98-
fi
99-
}
100-
10192
download_binary() {
102-
curl -R -z "$BINARY_ZIP_PATH" -L "http://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
93+
curl -R -z "$BINARY_ZIP_PATH" -L "https://api.browserstack.com/sdk/v1/download_cli?os=${OS}&os_arch=${ARCH}" -o "$BINARY_ZIP_PATH"
10394
bsdtar -xvf "$BINARY_ZIP_PATH" -O > "$BINARY_PATH" && chmod 0775 "$BINARY_PATH"
10495
}
10596

106-
script_self_update
10797
if [[ $SUBCOMMAND == "register-pre-commit-hook" ]]; then
10898
register_git_hook
10999
exit 0

0 commit comments

Comments
 (0)