diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml new file mode 100644 index 0000000..1833777 --- /dev/null +++ b/.github/workflows/php.yml @@ -0,0 +1,43 @@ +name: PHP + +on: + pull_request: + branches: ["master", "main"] + push: + branches: ["master", "main"] + +permissions: + contents: read + +jobs: + test: + name: lint + phpunit + runs-on: ubuntu-latest + # 7.4 rather than 8.x: lib/ sets properties dynamically, which PHP 8.2 + # deprecates, and the library's own floor is php >= 5.3.19. phpunit 9.6 + # supports 7.3+. + container: php:7.4-cli + steps: + - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3 + + # Runs first and on its own: a syntax error must fail the build even if the + # suite cannot boot. + - name: Syntax check + run: | + php -l lib/Local.php + php -l lib/LocalBinary.php + php -l lib/LocalException.php + php -l tests/LocalTest.php + php -l tests/LocalBinaryTest.php + + # git + unzip are not in the official php image, and Composer needs one of + # them to unpack downloaded packages (ext-zip is not built in either). + - name: Install dependencies + run: | + apt-get update -qq && apt-get install -y -qq --no-install-recommends git unzip >/dev/null + curl -sS https://getcomposer.org/installer | php + php composer.phar install --no-interaction --no-progress + + # Excludes @group network — those tests reach badssl.com and the real S3 host. + - name: PHPUnit + run: ./vendor/bin/phpunit --exclude-group network diff --git a/README.md b/README.md index 36c6a47..626a976 100644 --- a/README.md +++ b/README.md @@ -134,12 +134,45 @@ To specify the path to file where the logs will be saved - $bs_local_args = array("key" => "", "logfile" => "/browserstack/logs.txt"); ``` +## Upgrading + +### TLS verification on the binary download + +Earlier releases downloaded the BrowserStack Local binary with TLS certificate +verification disabled. **Verification is now enforced**, and the download also +rejects a response that is not a valid binary rather than storing it. + +If your machine sits behind a TLS-inspecting proxy whose certificate authority is +not in the system trust store, the download will now fail instead of silently +succeeding, and you will see: + +``` +Download failed (cURL error 60): SSL certificate problem: self signed certificate in certificate chain +``` + +**Remedy:** install your organisation's CA certificate into the system trust store +(libcurl honours it) — on Debian/Ubuntu, drop the PEM into +`/usr/local/share/ca-certificates/` and run `update-ca-certificates`; on macOS, add +it to the System keychain and mark it trusted; on Windows, import it into +*Trusted Root Certification Authorities*. Alternatively, download the binary out of +band and point the library at it with the `binaryPath` argument. + +We do not provide a switch to disable verification: it is what protects the binary +you are about to execute from being substituted in transit. + ## Contribute Testing is possible using [PHPUnit](https://phpunit.de/). To run the tests, run the command: `phpunit` +`@group network` tests reach external hosts. To skip them: +`phpunit --exclude-group network`. + +`tests/manual/php-poc.sh` is an end-to-end demonstration that the binary download +refuses a substituted binary served over an untrusted TLS certificate. It needs +`php`, `git`, `openssl` and `python3`, and is run by hand — not part of the suite. + ### Reporting bugs You can submit bug reports either in the Github issue tracker. diff --git a/composer.json b/composer.json index 165a2e2..0c7172c 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,7 @@ "php": ">=5.3.19" }, "require-dev": { - "phpunit/phpunit": "4.6.*" + "phpunit/phpunit": "^9.6" }, "suggest": { "phpdocumentor/phpdocumentor": "2.*" diff --git a/lib/LocalBinary.php b/lib/LocalBinary.php index 5c6121a..8153fe0 100644 --- a/lib/LocalBinary.php +++ b/lib/LocalBinary.php @@ -9,6 +9,14 @@ class LocalBinary { + const DOWNLOAD_ATTEMPTS = 3; + const CONNECT_TIMEOUT = 10; + const DOWNLOAD_TIMEOUT = 300; + + // Any real BrowserStackLocal build is tens of MB. A gateway error page or a + // truncated transfer is orders of magnitude smaller. + const MIN_BINARY_SIZE = 1048576; + public function __construct() { $this->possible_binary_paths = array( $this->server_home() . "/.browserstack", @@ -22,17 +30,18 @@ public function __destruct() { public function binary_path() { $dest_parent_dir = $this->get_available_dirs(); - $dest_binary_name = "BrowserStackLocal"; - if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') { - $dest_binary_name = $dest_binary_name. ".exe"; - } - $binary_path = $dest_parent_dir. "/". $dest_binary_name; - if(file_exists($binary_path)){ - return $binary_path; - } - else { - return $this->download_binary($dest_parent_dir); + $binary_path = $dest_parent_dir. "/". $this->dest_binary_name(); + if (file_exists($binary_path)) { + if ($this->verify_binary($binary_path)) { + $this->make_executable($binary_path); + return $binary_path; + } + // A cached file that is not a usable binary is discarded rather than + // executed — it may be a stored error page or a partial download. + // nosemgrep: php.lang.security.unlink-use.unlink-use -- basename is fixed by dest_binary_name(), no user input in the path + unlink($binary_path); } + return $this->download_binary($dest_parent_dir); } private function server_home() { @@ -52,10 +61,20 @@ private function server_home() { return empty($home) ? NULL : $home; } - private function platform_url(){ + private function is_windows() { + return strtoupper(substr(PHP_OS, 0, 3)) === 'WIN'; + } + + private function dest_binary_name() { + return $this->is_windows() ? "BrowserStackLocal.exe" : "BrowserStackLocal"; + } + + // protected so tests can point the download at a local fixture without + // patching installed source. + protected function platform_url(){ if (PHP_OS == "Darwin") return 'https://s3.amazonaws.com/browserStack/browserstack-local/BrowserStackLocal-darwin-x64'; - else if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') + else if ($this->is_windows()) return 'https://s3.amazonaws.com/browserStack/browserstack-local/BrowserStackLocal.exe'; if ((strtoupper(PHP_OS)) == "LINUX") { if (PHP_INT_SIZE * 8 == 64) @@ -67,27 +86,109 @@ private function platform_url(){ public function download_binary($path) { $url = $this->platform_url(); + if (empty($url)) + throw new LocalException("No BrowserStack Local binary is available for platform " . PHP_OS); + if (!file_exists($path)) mkdir($path, 0777, true); + $dest_binary_path = $path. '/'. $this->dest_binary_name(); + $last_error = "unknown error"; - $dest_binary_name = "BrowserStackLocal"; - if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') { - $dest_binary_name = $dest_binary_name. ".exe"; + for ($attempt = 1; $attempt <= self::DOWNLOAD_ATTEMPTS; $attempt++) { + try { + $this->fetch_binary($url, $dest_binary_path); + if ($this->verify_binary($dest_binary_path)) { + // Execute permission is granted only after the download has been + // verified, never before. + $this->make_executable($dest_binary_path); + return $dest_binary_path; + } + $last_error = "downloaded file is not a valid BrowserStackLocal binary"; + } + catch (LocalException $e) { + $last_error = $e->getMessage(); + } + // Never leave an unverified file behind for a later run to pick up and execute. + // nosemgrep: php.lang.security.unlink-use.unlink-use -- basename is fixed by dest_binary_name(), no user input in the path + if (file_exists($dest_binary_path)) + unlink($dest_binary_path); } - $dest_binary_path = $path. '/'. $dest_binary_name; - $file = fopen($dest_binary_path , "w+"); - $ch = curl_init(""); - curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false); + + throw new LocalException("Error trying to download BrowserStack Local binary from " . + $url . " after " . self::DOWNLOAD_ATTEMPTS . " attempts. Last error: " . $last_error); + } + + protected function fetch_binary($url, $dest_binary_path) { + $file = fopen($dest_binary_path, "w+"); + if ($file === false) + throw new LocalException("Unable to open " . $dest_binary_path . " for writing"); + + $ch = curl_init(); curl_setopt($ch, CURLOPT_URL, $url); - curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); + // The binary is executed on the user's machine, so the transport that + // delivers it must be authenticated: validate the certificate chain and + // that the certificate matches the host we asked for. + curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true); + curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2); + curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); + // A redirect must not be able to downgrade the transfer to plaintext. + curl_setopt($ch, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS); + curl_setopt($ch, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS); curl_setopt($ch, CURLOPT_FILE, $file); - $data = curl_exec ($ch); - curl_close ($ch); - + curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, self::CONNECT_TIMEOUT); + curl_setopt($ch, CURLOPT_TIMEOUT, self::DOWNLOAD_TIMEOUT); + $result = curl_exec($ch); + $curl_errno = curl_errno($ch); + $curl_error = curl_error($ch); + $http_status = curl_getinfo($ch, CURLINFO_HTTP_CODE); + curl_close($ch); fclose($file); - chmod($dest_binary_path, 0755); - return $dest_binary_path; + + if ($result === false || $curl_errno !== 0) + throw new LocalException("Download failed (cURL error " . $curl_errno . "): " . $curl_error); + + if ($http_status < 200 || $http_status >= 300) + throw new LocalException("Download failed with HTTP status " . $http_status); + } + + // Confirms the bytes on disk are a platform executable of plausible size. + // This catches error pages, truncated transfers and empty files; it is not an + // authenticity check — that is the job of TLS chain validation in + // fetch_binary(). The file is deliberately not executed to test it. + protected function verify_binary($binary_path) { + // The retry loop stats the same path up to DOWNLOAD_ATTEMPTS times, so a + // cached stat entry from a previous attempt must not decide this one. + clearstatcache(true, $binary_path); + if (!is_file($binary_path)) + return false; + if (filesize($binary_path) < self::MIN_BINARY_SIZE) + return false; + + $handle = fopen($binary_path, "rb"); + if ($handle === false) + return false; + $magic = fread($handle, 4); + fclose($handle); + if ($magic === false || strlen($magic) < 4) + return false; + + if ($this->is_windows()) + $valid = (substr($magic, 0, 2) === "MZ"); + else if (PHP_OS == "Darwin") + // Mach-O 64/32-bit little-endian, plus the universal ("fat") header. + $valid = in_array($magic, array("\xcf\xfa\xed\xfe", "\xce\xfa\xed\xfe", "\xca\xfe\xba\xbe"), true); + else + $valid = ($magic === "\x7f" . "ELF"); + + return $valid; + } + + private function make_executable($binary_path) { + clearstatcache(true, $binary_path); + if ($this->is_windows() || is_executable($binary_path)) + return true; + return @chmod($binary_path, 0755); } private function get_available_dirs() { diff --git a/phpunit.xml b/phpunit.xml index c5544cc..1abbab8 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,8 +1,11 @@ - + + - tests + tests - diff --git a/tests/LocalBinaryTest.php b/tests/LocalBinaryTest.php new file mode 100644 index 0000000..265097c --- /dev/null +++ b/tests/LocalBinaryTest.php @@ -0,0 +1,149 @@ +url = $url; + } + + protected function platform_url() { + return $this->url; + } + + public function call_verify_binary($path) { + return $this->verify_binary($path); + } +} + +class LocalBinaryTest extends \PHPUnit\Framework\TestCase { + + private $binary; + private $dir; + + protected function setUp(): void { + $this->binary = new TestableLocalBinary(); + $this->dir = sys_get_temp_dir() . '/bs-local-binary-test-' . getmypid() . '-' . mt_rand(); + mkdir($this->dir, 0777, true); + } + + protected function tearDown(): void { + foreach (glob($this->dir . '/*') as $file) { + unlink($file); + } + if (is_dir($this->dir)) + rmdir($this->dir); + } + + private function dest_path() { + $name = strtoupper(substr(PHP_OS, 0, 3)) === 'WIN' ? 'BrowserStackLocal.exe' : 'BrowserStackLocal'; + return $this->dir . '/' . $name; + } + + /** + * LOC-6740: the download must refuse a server whose certificate chain does + * not validate. Before the fix CURLOPT_SSL_VERIFYPEER was false, so this + * returned a path to attacker-supplied bytes instead of raising. + * + * @group network + */ + public function test_download_rejects_untrusted_certificate() { + $this->binary->set_url('https://self-signed.badssl.com/'); + $raised = null; + try { + $this->binary->download_binary($this->dir); + } + catch (LocalException $e) { + $raised = $e; + } + $this->assertNotNull($raised, 'download_binary must reject an untrusted certificate'); + // cURL error 60 is CURLE_PEER_FAILED_VERIFICATION — pins the failure to + // certificate validation rather than any later check. + $this->assertStringContainsString('cURL error 60', $raised->getMessage()); + $this->assertFalse(file_exists($this->dest_path()), 'no file may be left behind on failure'); + } + + /** + * @group network + */ + public function test_download_rejects_hostname_mismatch() { + $this->binary->set_url('https://wrong.host.badssl.com/'); + $raised = null; + try { + $this->binary->download_binary($this->dir); + } + catch (LocalException $e) { + $raised = $e; + } + $this->assertNotNull($raised, 'download_binary must reject a certificate for another host'); + $this->assertFalse(file_exists($this->dest_path())); + } + + /** + * A non-2xx response body must never be stored and made executable. + * + * @group network + */ + public function test_download_rejects_http_error_status() { + $this->binary->set_url('https://s3.amazonaws.com/browserStack/browserstack-local/does-not-exist'); + $raised = null; + try { + $this->binary->download_binary($this->dir); + } + catch (LocalException $e) { + $raised = $e; + } + $this->assertNotNull($raised, 'download_binary must reject a non-2xx response'); + $this->assertFalse(file_exists($this->dest_path())); + } + + public function test_verify_binary_rejects_error_page() { + $path = $this->dir . '/error-page'; + file_put_contents($path, 'AccessDenied'); + $this->assertFalse($this->binary->call_verify_binary($path)); + } + + public function test_verify_binary_rejects_empty_file() { + $path = $this->dir . '/empty'; + file_put_contents($path, ''); + $this->assertFalse($this->binary->call_verify_binary($path)); + } + + /** + * A file of plausible size that is not a platform executable — e.g. a large + * HTML interstitial from a captive portal — must still be rejected. + */ + public function test_verify_binary_rejects_large_non_executable() { + $path = $this->dir . '/large-html'; + file_put_contents($path, '' . str_repeat('a', 2 * 1024 * 1024) . ''); + $this->assertFalse($this->binary->call_verify_binary($path)); + } + + public function test_verify_binary_accepts_platform_executable() { + if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') + $magic = "MZ\x90\x00"; + else if (PHP_OS == 'Darwin') + $magic = "\xcf\xfa\xed\xfe"; + else + $magic = "\x7f" . "ELF"; + + $path = $this->dir . '/fake-binary'; + file_put_contents($path, $magic . str_repeat("\x00", 2 * 1024 * 1024)); + $this->assertTrue($this->binary->call_verify_binary($path)); + } +} + +?> diff --git a/tests/LocalTest.php b/tests/LocalTest.php index bee33df..b72994f 100644 --- a/tests/LocalTest.php +++ b/tests/LocalTest.php @@ -8,27 +8,27 @@ require_once __DIR__ . '/../vendor/autoload.php'; -class LocalTest extends \PHPUnit_Framework_TestCase { +class LocalTest extends \PHPUnit\Framework\TestCase { private $bs_local; - public function setUp(){ + protected function setUp(): void { $this->bs_local = new Local(); } - public function tearDown(){ + protected function tearDown(): void { $this->bs_local->stop(); } public function test_verbose() { $this->bs_local->add_args('v'); - $this->assertContains('-v',$this->bs_local->start_command()); + $this->assertStringContainsString('-v',$this->bs_local->start_command()); } public function test_set_folder() { $this->bs_local->add_args('f', "/"); - $this->assertContains('-f',$this->bs_local->start_command()); - $this->assertContains('/',$this->bs_local->start_command()); + $this->assertStringContainsString('-f',$this->bs_local->start_command()); + $this->assertStringContainsString('/',$this->bs_local->start_command()); } public function test_enable_force() { @@ -37,36 +37,36 @@ public function test_enable_force() { public function test_set_local_identifier() { $this->bs_local->add_args("localIdentifier", "randomString"); - $this->assertContains('-localIdentifier randomString',$this->bs_local->start_command()); + $this->assertStringContainsString('-localIdentifier randomString',$this->bs_local->start_command()); } public function test_enable_only() { $this->bs_local->add_args("only"); - $this->assertContains('-only',$this->bs_local->start_command()); + $this->assertStringContainsString('-only',$this->bs_local->start_command()); } public function test_enable_only_automate() { $this->bs_local->add_args("onlyAutomate"); - $this->assertContains('-onlyAutomate', $this->bs_local->start_command()); + $this->assertStringContainsString('-onlyAutomate', $this->bs_local->start_command()); } public function test_enable_force_local() { $this->bs_local->add_args("forcelocal"); - $this->assertContains('-forcelocal',$this->bs_local->start_command()); + $this->assertStringContainsString('-forcelocal',$this->bs_local->start_command()); } public function test_custom_boolean_argument() { $this->bs_local->add_args("boolArg1", true); $this->bs_local->add_args("boolArg2", true); - $this->assertContains('-boolArg1',$this->bs_local->start_command()); - $this->assertContains('-boolArg2',$this->bs_local->start_command()); + $this->assertStringContainsString('-boolArg1',$this->bs_local->start_command()); + $this->assertStringContainsString('-boolArg2',$this->bs_local->start_command()); } public function test_custom_keyval() { $this->bs_local->add_args("customKey1", "custom value1"); $this->bs_local->add_args("customKey2", "custom value2"); - $this->assertContains('-customKey1 \'custom value1\'',$this->bs_local->start_command()); - $this->assertContains('-customKey2 \'custom value2\'',$this->bs_local->start_command()); + $this->assertStringContainsString('-customKey1 \'custom value1\'',$this->bs_local->start_command()); + $this->assertStringContainsString('-customKey2 \'custom value2\'',$this->bs_local->start_command()); } public function test_set_proxy() { @@ -74,19 +74,24 @@ public function test_set_proxy() { $this->bs_local->add_args("proxyPort", 8080); $this->bs_local->add_args("proxyUser", "user"); $this->bs_local->add_args("proxyPass", "pass"); - $this->assertContains('-proxyHost localhost -proxyPort 8080 -proxyUser user -proxyPass pass',$this->bs_local->start_command()); + $this->assertStringContainsString('-proxyHost localhost -proxyPort 8080 -proxyUser user -proxyPass pass',$this->bs_local->start_command()); } public function test_enable_force_proxy() { $this->bs_local->add_args("-forceproxy"); - $this->assertContains('-forceproxy',$this->bs_local->start_command()); + $this->assertStringContainsString('-forceproxy',$this->bs_local->start_command()); } public function test_hosts() { $this->bs_local->add_args("-hosts", "localhost,8080,0"); - $this->assertContains('localhost,8080,0',$this->bs_local->start_command()); + $this->assertStringContainsString('localhost,8080,0',$this->bs_local->start_command()); } + /** + * Starts the real binary — needs BROWSERSTACK_ACCESS_KEY and outbound network. + * + * @group network + */ public function test_isRunning() { $this->assertFalse($this->bs_local->isRunning()); $this->bs_local->start(array('v' => true)); @@ -97,12 +102,22 @@ public function test_isRunning() { $this->assertTrue($this->bs_local->isRunning()); } + /** + * Starts the real binary — needs BROWSERSTACK_ACCESS_KEY and outbound network. + * + * @group network + */ public function test_checkPid() { $this->assertFalse($this->bs_local->isRunning()); $this->bs_local->start(array('v' => true)); $this->assertTrue($this->bs_local->pid > 0); } + /** + * Starts the real binary twice — needs BROWSERSTACK_ACCESS_KEY and outbound network. + * + * @group network + */ public function test_multiple_binary() { $this->bs_local->start(array('v' => true)); $bs_local_2 = new Local(); diff --git a/tests/manual/php-poc.sh b/tests/manual/php-poc.sh new file mode 100755 index 0000000..8c87dc5 --- /dev/null +++ b/tests/manual/php-poc.sh @@ -0,0 +1,144 @@ +#!/usr/bin/env bash +# +# LOC-6740 — manual MITM proof for LocalBinary's binary download. +# +# The automated suite (tests/LocalBinaryTest.php, run by .github/workflows/php.yml) +# covers the assertions. This harness is the end-to-end demonstration: it stands up +# a local HTTPS server with a self-signed certificate and runs the real +# LocalBinary class against it, on both the pre-fix and post-fix source. +# +# ARM 1 pre-fix source (git master), URL pointed at the MITM server +# -> expected ACCEPTED: the substituted payload is written and chmod 0755'd +# ARM 2 this working tree, same server +# -> expected REJECTED with cURL error 60, nothing left on disk +# ARM 3 this working tree, real platform URL, no override +# -> expected ACCEPTED: the genuine 36-40 MB binary, mode 755 +# +# ARM 3 needs outbound network. ARM 1 needs a local `master` ref. +# +# Requires: php (>= 5.3.19, with the curl and openssl extensions), git, openssl, python3. +# +# Usage: +# tests/manual/php-poc.sh # from anywhere inside the checkout +# REPO=/path/to/checkout tests/manual/php-poc.sh +# +set -u + +REPO="${REPO:-$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel 2>/dev/null)}" +[ -n "$REPO" ] && [ -f "$REPO/lib/LocalBinary.php" ] || { + echo "Could not locate the checkout. Run from inside it, or set REPO=/path/to/browserstack-local-php." >&2 + exit 1 +} +PORT="${PORT:-18443}" +WORK="$(mktemp -d)" +SRV_PID="" +cleanup() { [ -n "$SRV_PID" ] && kill "$SRV_PID" 2>/dev/null; rm -rf "$WORK"; } +trap cleanup EXIT + +for tool in php git openssl python3; do + command -v "$tool" >/dev/null || { echo "$tool not found" >&2; exit 1; } +done +php -r 'exit(extension_loaded("curl") && extension_loaded("openssl") ? 0 : 1);' || { + echo "php is missing the curl and/or openssl extension" >&2; exit 1; } +echo "php: $(php -r 'echo PHP_VERSION;') repo: $REPO" + +# A self-signed certificate for localhost. The chain does not validate, which is +# exactly what CURLOPT_SSL_VERIFYPEER controls. Using localhost rather than +# s3.amazonaws.com avoids needing /etc/hosts or curl --resolve, neither of which +# can be injected into the class under test. +openssl req -x509 -newkey rsa:2048 -nodes -days 1 \ + -keyout "$WORK/key.pem" -out "$WORK/cert.pem" \ + -subj "/CN=localhost" -addext "subjectAltName=DNS:localhost" 2>/dev/null +echo "attacker cert: $(openssl x509 -in "$WORK/cert.pem" -noout -subject -issuer | tr '\n' ' ')" + +# The payload is > 1 MiB and carries ELF magic so it also satisfies verify_binary()'s +# format/size gate. That isolates these arms to the TLS control alone. +mkdir -p "$WORK/root" +printf '\x7fELF' > "$WORK/root/BrowserStackLocal-linux-x64" +head -c 2097152 /dev/zero >> "$WORK/root/BrowserStackLocal-linux-x64" +PAYLOAD_URL="https://localhost:$PORT/BrowserStackLocal-linux-x64" + +python3 - "$WORK" "$PORT" <<'PY' & +import http.server, os, ssl, sys +work, port = sys.argv[1], int(sys.argv[2]) +os.chdir(os.path.join(work, "root")) +ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) +ctx.load_cert_chain(os.path.join(work, "cert.pem"), os.path.join(work, "key.pem")) +srv = http.server.HTTPServer(("127.0.0.1", port), http.server.SimpleHTTPRequestHandler) +srv.socket = ctx.wrap_socket(srv.socket, server_side=True) +srv.serve_forever() +PY +SRV_PID=$! +for _ in $(seq 1 40); do + openssl s_client -connect "127.0.0.1:$PORT" /dev/null 2>&1 && break + sleep 0.25 +done + +# Runs a given LocalBinary.php with every hardcoded platform URL rewritten to the +# local payload URL. The sed covers both the pre-fix (private) and post-fix +# (protected) shape of platform_url(), so no subclass is needed. +run_arm() { + local src="$1" out="$2" name="$3" + mkdir -p "$out/dest" + sed -E "s#https://s3\.amazonaws\.com/browserStack/browserstack-local/[A-Za-z0-9._-]+#$PAYLOAD_URL#g" \ + "$src" > "$out/LocalBinary.php" + cp "$REPO/lib/LocalException.php" "$out/LocalException.php" + cat > "$out/run.php" <download_binary('$out/dest'); + if (\$path && file_exists(\$path)) { + printf("ACCEPTED: %s (%d bytes, mode %o)\n", \$path, filesize(\$path), fileperms(\$path) & 0777); + echo "VERDICT[$name]: attacker bytes ACCEPTED and made executable\n"; + } else { + echo "VERDICT[$name]: INCONCLUSIVE — returned \$path with no file\n"; + } +} catch (LocalException \$e) { + echo "REJECTED: " . \$e->getMessage() . "\n"; + \$left = glob('$out/dest/BrowserStackLocal*'); + echo "leftover files: " . (count(\$left) ? implode(',', \$left) : 'none') . "\n"; + echo "VERDICT[$name]: substitution REFUSED, nothing left on disk\n"; +} +PHP + php "$out/run.php" +} + +echo +echo "================ ARM 1 — pre-fix source (git master) ================" +if git -C "$REPO" show master:lib/LocalBinary.php > "$WORK/prefix.php" 2>/dev/null; then + grep -n 'VERIFYPEER' "$WORK/prefix.php" || echo "(no VERIFYPEER line found)" + run_arm "$WORK/prefix.php" "$WORK/arm1" "pre-fix" +else + echo "SKIPPED — no local 'master' ref (git fetch origin master:master to enable)" +fi + +echo +echo "================ ARM 2 — this working tree, MITM server ============" +grep -n 'VERIFYPEER\|VERIFYHOST' "$REPO/lib/LocalBinary.php" +run_arm "$REPO/lib/LocalBinary.php" "$WORK/arm2" "post-fix-negative" + +echo +echo "================ ARM 3 — this working tree, real download ==========" +mkdir -p "$WORK/arm3/dest" +php -r " +require '$REPO/lib/LocalException.php'; +require '$REPO/lib/LocalBinary.php'; +use BrowserStack\LocalBinary; +use BrowserStack\LocalException; +try { + \$b = new LocalBinary(); + \$p = \$b->download_binary('$WORK/arm3/dest'); + printf(\"ACCEPTED: %s (%d bytes, mode %o)\n\", \$p, filesize(\$p), fileperms(\$p) & 0777); + echo \"VERDICT[happy-path]: OK — the real binary still downloads and verifies\n\"; +} catch (LocalException \$e) { + echo 'FAILED: ' . \$e->getMessage() . \"\n\"; + echo \"VERDICT[happy-path]: REGRESSION — the fix broke the legitimate download\n\"; +}" + +echo +echo "Expected: ARM 1 ACCEPTED (vulnerable), ARM 2 REFUSED (cURL error 60), ARM 3 OK."