55
66using NUnit . Framework ;
77using BrowserStack ;
8+ using System . Collections . Generic ;
89using System . Text ;
910using System . IO ;
1011
@@ -91,15 +92,15 @@ public void TestBinaryPathOnNoMoreFallback()
9192 public void TestBinaryArguments ( )
9293 {
9394 tunnel = new TunnelClass ( ) ;
94- tunnel . addBinaryArguments ( "dummyArguments" ) ;
95- Assert . AreEqual ( tunnel . getBinaryArguments ( ) , "dummyArguments" ) ;
95+ tunnel . addBinaryArguments ( new List < string > { "-dummyFlag" , "dummyValue" } ) ;
96+ CollectionAssert . AreEqual ( new List < string > { "-dummyFlag" , "dummyValue" } , tunnel . getBinaryArguments ( ) ) ;
9697 }
9798 [ TestMethod ]
9899 public void TestBinaryArgumentsAreEmptyOnNull ( )
99100 {
100101 tunnel = new TunnelClass ( ) ;
101102 tunnel . addBinaryArguments ( null ) ;
102- Assert . AreEqual ( tunnel . getBinaryArguments ( ) , "" ) ;
103+ Assert . IsEmpty ( tunnel . getBinaryArguments ( ) ) ;
103104 }
104105
105106
@@ -130,9 +131,74 @@ public void testFallbackException()
130131 {
131132 tunnel . fallbackPaths ( ) ;
132133 }
134+
135+ // Regression for the chmod shell-metacharacter injection (F-001): binaryAbsolute must
136+ // reach chmod as a single argument, never interpolated into a shell command line. On
137+ // pre-fix code (`bash -c "chmod 0755 <path>"`) the payload below runs `touch <marker>`
138+ // and never chmods the real file, so BOTH asserts fail; the fix (`/bin/chmod` +
139+ // ArgumentList) creates no marker and chmods the real path. Unix-only: on Windows
140+ // modifyBinaryPermission takes the ACL branch, not chmod.
141+ [ TestMethod ]
142+ public void TestModifyBinaryPermissionDoesNotInterpretShellMetacharacters ( )
143+ {
144+ if ( os . Platform . ToString ( ) != "Unix" )
145+ {
146+ Assert . Ignore ( "Unix-only: Windows takes the ACL branch in modifyBinaryPermission, not chmod" ) ;
147+ return ;
148+ }
149+
150+ string prevCwd = Directory . GetCurrentDirectory ( ) ;
151+ // Space-free working dir so the injected `touch pwned` (if it runs) lands here deterministically.
152+ string work = Path . Combine ( Path . GetTempPath ( ) , "bsloc" + Guid . NewGuid ( ) . ToString ( "N" ) ) ;
153+ Directory . CreateDirectory ( work ) ;
154+ Directory . SetCurrentDirectory ( work ) ;
155+ try
156+ {
157+ // Filename carries a space AND a shell-injection payload. A filename cannot contain '/',
158+ // so the injected command targets the (deterministic) CWD, not an absolute path.
159+ string binaryPath = Path . Combine ( work , "bs local; touch pwned; #" ) ;
160+ File . WriteAllText ( binaryPath , "#!/bin/sh\n " ) ; // default perms ~0644 (not executable)
161+
162+ tunnel = new TunnelClass ( ) ;
163+ ( ( TunnelClass ) tunnel ) . setBinaryAbsolute ( binaryPath ) ;
164+ tunnel . modifyBinaryPermission ( ) ;
165+
166+ Assert . IsFalse ( File . Exists ( Path . Combine ( work , "pwned" ) ) ,
167+ "shell metacharacters in binaryAbsolute were interpreted - OS command injection" ) ;
168+ Assert . IsTrue ( IsExecutable ( binaryPath ) ,
169+ "chmod 0755 was not applied to the real binary path (the path was mangled by the shell)" ) ;
170+ }
171+ finally
172+ {
173+ Directory . SetCurrentDirectory ( prevCwd ) ;
174+ try { Directory . Delete ( work , true ) ; } catch { }
175+ }
176+ }
177+
178+ // Returns true iff `path` has the execute bit set. Uses sh's `$0` positional so the
179+ // path (which contains a space + metacharacters) is passed safely, not re-parsed.
180+ private static bool IsExecutable ( string path )
181+ {
182+ var psi = new System . Diagnostics . ProcessStartInfo ( "/bin/sh" ) { UseShellExecute = false } ;
183+ psi . ArgumentList . Add ( "-c" ) ;
184+ psi . ArgumentList . Add ( "test -x \" $0\" " ) ;
185+ psi . ArgumentList . Add ( path ) ;
186+ using ( var p = System . Diagnostics . Process . Start ( psi ) )
187+ {
188+ p . WaitForExit ( ) ;
189+ return p . ExitCode == 0 ;
190+ }
191+ }
133192 public class TunnelClass : BrowserStackTunnel
134193 {
135194 public TunnelClass ( ) : base ( "test-user-agent" ) { }
195+ // Stub the network boundary so these binary-path/fallback unit tests exercise
196+ // the real path-resolution logic without making a live HTTP call to the
197+ // endpoint API (which addBinaryPath triggers on first invocation).
198+ protected override string fetchSourceUrl ( string accessKey )
199+ {
200+ return null ;
201+ }
136202 public StringBuilder getOutputBuilder ( )
137203 {
138204 return output ;
@@ -141,10 +207,14 @@ public string getBinaryAbsolute()
141207 {
142208 return binaryAbsolute ;
143209 }
144- public string getBinaryArguments ( )
210+ public List < string > getBinaryArguments ( )
145211 {
146212 return binaryArguments ;
147213 }
214+ public void setBinaryAbsolute ( string path )
215+ {
216+ binaryAbsolute = path ;
217+ }
148218 }
149219 }
150220}
0 commit comments