Skip to content

Commit e766c21

Browse files
committed
test: pin the array-form behaviour with a real binary at a hostile path
The existing injection test returns false post-fix via Errno::ENOENT rather than by demonstrating that the named file is executed verbatim, so it would also pass under a character-allowlist remediation instead of the array form. Putting the metacharacters in the directory name of a real, executable script asserts both halves at once: the injected command never runs, and the legitimate binary at that hostile-looking path still verifies. This is the same shape as the reproduction that exercises the vulnerability through $HOME. Verified to fail before the fix on the injection assertion.
1 parent ab48704 commit e766c21

1 file changed

Lines changed: 25 additions & 0 deletions

File tree

test/browserstack-local-test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,31 @@ def test_verify_binary_does_not_interpret_shell_metacharacters_in_path
172172
File.delete(marker) if marker && File.exist?(marker)
173173
end
174174

175+
# Stronger form of the above: a REAL binary living under a hostile-looking
176+
# directory name. Pins the array-form behaviour itself rather than just an
177+
# ENOENT, so a future "fix" that swapped the array form for a character
178+
# allowlist would fail here — the injected command must not run AND the
179+
# legitimate binary at that path must still verify.
180+
def test_verify_binary_runs_a_real_binary_at_a_path_containing_shell_metacharacters
181+
skip 'needs a POSIX shell to stand in for the binary' if Gem.win_platform?
182+
183+
marker = File.join(Dir.tmpdir, "bs_local_verify_dir_injection_#{Process.pid}")
184+
File.delete(marker) if File.exist?(marker)
185+
186+
base = Dir.mktmpdir('bs_local')
187+
dir = File.join(base, "h;touch #{marker};echo BrowserStack Local version 9.9;#")
188+
FileUtils.mkdir_p(dir)
189+
bin = File.join(dir, 'BrowserStackLocal')
190+
File.write(bin, "#!/bin/sh\necho 'BrowserStack Local version 9.9'\n")
191+
FileUtils.chmod(0755, bin)
192+
193+
assert_equal true, BrowserStack::LocalBinary.new(auth_token: 'fake').send(:verify_binary, bin)
194+
refute File.exist?(marker), 'shell metacharacters in the binary path were executed'
195+
ensure
196+
File.delete(marker) if marker && File.exist?(marker)
197+
FileUtils.remove_entry(base) if base && File.directory?(base)
198+
end
199+
175200
# Same fix, benign side: a legitimate path containing spaces must still verify
176201
# (the shell used to split it and the check failed for every such user).
177202
def test_verify_binary_accepts_a_path_containing_spaces

0 commit comments

Comments
 (0)