From 08c09394cef979627890ad9278d62ee69b0a2951 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Fri, 22 May 2026 15:58:59 +0530 Subject: [PATCH 1/6] fixed logfile clear logic --- browserstack/local.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/browserstack/local.py b/browserstack/local.py index 544c791..87ab146 100644 --- a/browserstack/local.py +++ b/browserstack/local.py @@ -93,7 +93,14 @@ def start(self, **kwargs): self.proc = subprocess.Popen(self._generate_cmd(), stdout=subprocess.PIPE, stderr=subprocess.PIPE) (out, err) = self.proc.communicate() - os.system('echo "" > "'+ self.local_logfile_path +'"') + logfile_dir = os.path.dirname(self.local_logfile_path) + if logfile_dir: + os.makedirs(logfile_dir, exist_ok=True) + try: + with open(self.local_logfile_path, 'w') as f: + f.write('') + except OSError as e: + raise BrowserStackLocalError('Unable to open logfile: {}'.format(e)) try: if out: output_string = out.decode() From e89bb01ea27fee7d599c73a3cdf01408dbfa7b72 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Fri, 22 May 2026 16:41:26 +0530 Subject: [PATCH 2/6] fixes for binarypath arg --- browserstack/local.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/browserstack/local.py b/browserstack/local.py index 87ab146..d4da3b9 100644 --- a/browserstack/local.py +++ b/browserstack/local.py @@ -1,4 +1,4 @@ -import subprocess, os, time, json,logging +import subprocess, os, time, json, logging, re import psutil from browserstack.local_binary import LocalBinary @@ -70,7 +70,16 @@ def start(self, **kwargs): del self.options['key'] if 'binarypath' in self.options: - self.binary_path = self.options['binarypath'] + candidate = os.path.realpath(self.options['binarypath']) + if not os.path.isfile(candidate): + raise BrowserStackLocalError('binarypath does not point to a file') + try: + version_output = subprocess.check_output([candidate, '--version']).decode('utf-8') + except (subprocess.SubprocessError, OSError) as e: + raise BrowserStackLocalError('binarypath failed verification: {}'.format(e)) + if not re.match(r'BrowserStack Local version \d+\.\d+', version_output): + raise BrowserStackLocalError('binarypath failed verification') + self.binary_path = candidate del self.options['binarypath'] else: l = LocalBinary(self.key) From b5bb4f777b0c4b9044b4dd9dcb951f5f78e3617e Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Fri, 22 May 2026 18:27:06 +0530 Subject: [PATCH 3/6] fixes for LOC-6720 --- .github/workflows/Semgrep.yml | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/Semgrep.yml b/.github/workflows/Semgrep.yml index 0347afd..80b376b 100644 --- a/.github/workflows/Semgrep.yml +++ b/.github/workflows/Semgrep.yml @@ -27,7 +27,7 @@ jobs: container: # A Docker image with Semgrep installed. Do not change this. - image: returntocorp/semgrep + image: returntocorp/semgrep@sha256:9349edbadf90c3f3c0c3f55867625354e89680e6fa10d9034042af52fdb0e0d0 # Skip any PR created by dependabot to avoid permission issues: if: (github.actor != 'dependabot[bot]') diff --git a/setup.py b/setup.py index 7cf8cc6..6af15db 100644 --- a/setup.py +++ b/setup.py @@ -16,7 +16,7 @@ keywords = ['BrowserStack', 'Local', 'selenium', 'testing'], classifiers = [], install_requires=[ - 'psutil', + 'psutil>=5.6.6,<7', ], ) From cb7613959de4eeef501e061164d0141ba6908f5b Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Mon, 25 May 2026 17:18:05 +0530 Subject: [PATCH 4/6] minor fixes --- browserstack/local.py | 11 ++++++----- browserstack/local_binary.py | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/browserstack/local.py b/browserstack/local.py index d4da3b9..5d07bb0 100644 --- a/browserstack/local.py +++ b/browserstack/local.py @@ -74,10 +74,10 @@ def start(self, **kwargs): if not os.path.isfile(candidate): raise BrowserStackLocalError('binarypath does not point to a file') try: - version_output = subprocess.check_output([candidate, '--version']).decode('utf-8') + version_output = subprocess.check_output([candidate, '--version'], timeout=10).decode('utf-8') except (subprocess.SubprocessError, OSError) as e: raise BrowserStackLocalError('binarypath failed verification: {}'.format(e)) - if not re.match(r'BrowserStack Local version \d+\.\d+', version_output): + if not re.match(LocalBinary.VERSION_REGEX, version_output): raise BrowserStackLocalError('binarypath failed verification') self.binary_path = candidate del self.options['binarypath'] @@ -99,9 +99,6 @@ def start(self, **kwargs): if 'source' in self.options: del self.options['source'] - self.proc = subprocess.Popen(self._generate_cmd(), stdout=subprocess.PIPE, stderr=subprocess.PIPE) - (out, err) = self.proc.communicate() - logfile_dir = os.path.dirname(self.local_logfile_path) if logfile_dir: os.makedirs(logfile_dir, exist_ok=True) @@ -110,6 +107,10 @@ def start(self, **kwargs): f.write('') except OSError as e: raise BrowserStackLocalError('Unable to open logfile: {}'.format(e)) + + self.proc = subprocess.Popen(self._generate_cmd(), stdout=subprocess.PIPE, stderr=subprocess.PIPE) + (out, err) = self.proc.communicate() + try: if out: output_string = out.decode() diff --git a/browserstack/local_binary.py b/browserstack/local_binary.py index 13144fa..2fa7398 100644 --- a/browserstack/local_binary.py +++ b/browserstack/local_binary.py @@ -9,6 +9,7 @@ from urllib2 import urlopen, Request class LocalBinary: + VERSION_REGEX = r"BrowserStack Local version \d+\.\d+" _version = None def __init__(self, key, error_object=None): @@ -159,8 +160,7 @@ def read_chunk(chunk_size): def __verify_binary(self,path): try: binary_response = subprocess.check_output([path,"--version"]).decode("utf-8") - pattern = re.compile("BrowserStack Local version \d+\.\d+") - return bool(pattern.match(binary_response)) + return bool(re.match(LocalBinary.VERSION_REGEX, binary_response)) except: return False From 6238d1ec5718968250e434dff464fa403ac67eb7 Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Tue, 26 May 2026 10:57:18 +0530 Subject: [PATCH 5/6] added allowlist of args/flags --- browserstack/local.py | 67 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/browserstack/local.py b/browserstack/local.py index 5d07bb0..606a8a7 100644 --- a/browserstack/local.py +++ b/browserstack/local.py @@ -10,6 +10,71 @@ except: import pkg_resources +# LOC-6719 / INJ-002 (CWE-88): kwargs are forwarded to the BrowserStackLocal +# binary as flags. Without this allowlist an attacker-influenced kwarg can +# inject arbitrary flags — notably proxyHost / proxyPort / forceproxy to +# redirect tunnel traffic through an attacker-controlled proxy. +# +# Cross-referenced against the binary's COMMAND_CONFIGURATION table +# (browserStackTunnel/extensions/node/config/constants.js) and the public docs +# at https://www.browserstack.com/docs/local-testing/binary-params. Both the +# camelCase form ('name') and the kebab-case form ('alias') are accepted +# because user code passes both shapes. Internal-only flags (visible:false: +# region, bsHost, customRepeater, enterprise, public-interface-services, +# identifier, trusted-hosts), help/version flags, and undocumented internal +# flags (-r, -skipCheck, -daemonInstance, -tunnelIdentifier, -uniqueIdentifier) +# are intentionally excluded. +# +# Wrapper-managed keys (key, binarypath, logfile, source) are stripped from +# self.options in start() before this check runs and are not listed here. +# 'daemon' / 'logFile' are also omitted because they are emitted unconditionally +# by _generate_cmd and a user-supplied duplicate would conflict. +ALLOWED_OPTIONS = frozenset({ + # Verbose / logging + 'v', 'vv', 'vvv', 'verbose', + 'enableLoggingForAPI', 'enable-logging-for-api', + 'enableUTCLogging', 'enable-utc-logging', + # Folder testing + 'f', 'folder', + # Force / start behaviour + 'force', 'F', + 'forcelocal', 'force-local', 'forceLocal', + 'forceproxy', 'force-proxy', 'forceProxy', + 'onlyAutomate', 'only-automate', + # Host targeting / restriction + 'only', + 'include-hosts', 'exclude-hosts', + 'localIdentifier', 'local-identifier', + 'parallelRuns', 'parallel-runs', + # Corporate proxy + 'proxyHost', 'proxy-host', + 'proxyPort', 'proxy-port', + 'proxyUser', 'proxy-user', + 'proxyPass', 'proxy-pass', + 'disableProxyDiscovery', 'disable-proxy-discovery', + # Local proxy + 'localProxyHost', 'local-proxy-host', + 'localProxyPort', 'local-proxy-port', + 'localProxyUser', 'local-proxy-user', + 'localProxyPass', 'local-proxy-pass', + # PAC / HTTPS / protocol + 'pacFile', 'pac-file', + 'https-ports', + 'client-protocol', + # CA certificates + 'useCaCertificate', 'use-ca-certificate', + 'useSystemInstalledCa', 'use-system-installed-ca', + # NTLM proxy + 'ntlm-username', 'ntlm-password', 'ntlm-domain', 'ntlm-workstation', + # Dashboard / misc + 'disableDashboard', 'disable-dashboard', + 'config-file', + 'no-container', + 'connect-timeout', + 'timeout', + 'debug-utility', 'debug-url', +}) + class Local: def __init__(self, key=None, binary_path=None, **kwargs): self.key = os.environ['BROWSERSTACK_ACCESS_KEY'] if 'BROWSERSTACK_ACCESS_KEY' in os.environ else key @@ -52,6 +117,8 @@ def get_package_version(self): def _generate_cmd(self): cmd = [self.binary_path, '-d', 'start', '-logFile', self.local_logfile_path, "-k", self.key, '--source', 'python:' + self.get_package_version()] for o in self.options.keys(): + if o not in ALLOWED_OPTIONS: + raise BrowserStackLocalError('Unknown option: {}'.format(o)) if self.options.get(o) is not None: cmd = cmd + self.__xstr(o, self.options.get(o)) return cmd From dd6632842e979ab082dd961a0024f6f92071687b Mon Sep 17 00:00:00 2001 From: rounak bhatia Date: Tue, 26 May 2026 12:07:52 +0530 Subject: [PATCH 6/6] revert "added allowlist of args/flag" --- browserstack/local.py | 67 ------------------------------------------- 1 file changed, 67 deletions(-) diff --git a/browserstack/local.py b/browserstack/local.py index 606a8a7..5d07bb0 100644 --- a/browserstack/local.py +++ b/browserstack/local.py @@ -10,71 +10,6 @@ except: import pkg_resources -# LOC-6719 / INJ-002 (CWE-88): kwargs are forwarded to the BrowserStackLocal -# binary as flags. Without this allowlist an attacker-influenced kwarg can -# inject arbitrary flags — notably proxyHost / proxyPort / forceproxy to -# redirect tunnel traffic through an attacker-controlled proxy. -# -# Cross-referenced against the binary's COMMAND_CONFIGURATION table -# (browserStackTunnel/extensions/node/config/constants.js) and the public docs -# at https://www.browserstack.com/docs/local-testing/binary-params. Both the -# camelCase form ('name') and the kebab-case form ('alias') are accepted -# because user code passes both shapes. Internal-only flags (visible:false: -# region, bsHost, customRepeater, enterprise, public-interface-services, -# identifier, trusted-hosts), help/version flags, and undocumented internal -# flags (-r, -skipCheck, -daemonInstance, -tunnelIdentifier, -uniqueIdentifier) -# are intentionally excluded. -# -# Wrapper-managed keys (key, binarypath, logfile, source) are stripped from -# self.options in start() before this check runs and are not listed here. -# 'daemon' / 'logFile' are also omitted because they are emitted unconditionally -# by _generate_cmd and a user-supplied duplicate would conflict. -ALLOWED_OPTIONS = frozenset({ - # Verbose / logging - 'v', 'vv', 'vvv', 'verbose', - 'enableLoggingForAPI', 'enable-logging-for-api', - 'enableUTCLogging', 'enable-utc-logging', - # Folder testing - 'f', 'folder', - # Force / start behaviour - 'force', 'F', - 'forcelocal', 'force-local', 'forceLocal', - 'forceproxy', 'force-proxy', 'forceProxy', - 'onlyAutomate', 'only-automate', - # Host targeting / restriction - 'only', - 'include-hosts', 'exclude-hosts', - 'localIdentifier', 'local-identifier', - 'parallelRuns', 'parallel-runs', - # Corporate proxy - 'proxyHost', 'proxy-host', - 'proxyPort', 'proxy-port', - 'proxyUser', 'proxy-user', - 'proxyPass', 'proxy-pass', - 'disableProxyDiscovery', 'disable-proxy-discovery', - # Local proxy - 'localProxyHost', 'local-proxy-host', - 'localProxyPort', 'local-proxy-port', - 'localProxyUser', 'local-proxy-user', - 'localProxyPass', 'local-proxy-pass', - # PAC / HTTPS / protocol - 'pacFile', 'pac-file', - 'https-ports', - 'client-protocol', - # CA certificates - 'useCaCertificate', 'use-ca-certificate', - 'useSystemInstalledCa', 'use-system-installed-ca', - # NTLM proxy - 'ntlm-username', 'ntlm-password', 'ntlm-domain', 'ntlm-workstation', - # Dashboard / misc - 'disableDashboard', 'disable-dashboard', - 'config-file', - 'no-container', - 'connect-timeout', - 'timeout', - 'debug-utility', 'debug-url', -}) - class Local: def __init__(self, key=None, binary_path=None, **kwargs): self.key = os.environ['BROWSERSTACK_ACCESS_KEY'] if 'BROWSERSTACK_ACCESS_KEY' in os.environ else key @@ -117,8 +52,6 @@ def get_package_version(self): def _generate_cmd(self): cmd = [self.binary_path, '-d', 'start', '-logFile', self.local_logfile_path, "-k", self.key, '--source', 'python:' + self.get_package_version()] for o in self.options.keys(): - if o not in ALLOWED_OPTIONS: - raise BrowserStackLocalError('Unknown option: {}'.format(o)) if self.options.get(o) is not None: cmd = cmd + self.__xstr(o, self.options.get(o)) return cmd