Merge pull request #61 from browserstack/python_binding_security_issues

Fixes for security issues

Author: AdityaHirapara

.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]')

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'], timeout=10).decode('utf-8')
+      except (subprocess.SubprocessError, OSError) as e:
+        raise BrowserStackLocalError('binarypath failed verification: {}'.format(e))
+      if not re.match(LocalBinary.VERSION_REGEX, version_output):
+        raise BrowserStackLocalError('binarypath failed verification')
+      self.binary_path = candidate
       del self.options['binarypath']
     else:
       l = LocalBinary(self.key)
@@ -90,10 +99,18 @@ def start(self, **kwargs):
     if 'source' in self.options:
       del self.options['source']
 
+    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))
+
     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 +'"')
     try:
       if out:
         output_string = out.decode()

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
 

setup.py

@@ -5,7 +5,7 @@
 setup(
   name = 'browserstack-local',
   packages = ['browserstack'],
-  version = '1.2.14',
+  version = '1.2.15',
   description = 'Python bindings for Browserstack Local',
   long_description=open('README.md').read(),
   long_description_content_type='text/markdown',
@@ -16,7 +16,7 @@
   keywords = ['BrowserStack', 'Local', 'selenium', 'testing'],
   classifiers = [],
   install_requires=[
-        'psutil',
+        'psutil>=5.6.6,<7',
   ],
 )