Skip to content

Commit 159c132

Browse files
fix(security): validate JVM-property base URL and canonicalize uploadApp path [APS-19024]
INJ-002 (CWE-918 SSRF): both AutomateClient and AppAutomateClient constructors read the API base URL from a JVM system property (browserstack.automate.api / browserstack.app-automate.api) without validation. An attacker who can set the property redirects every signed request to their host and harvests Basic Auth credentials. Each client now wraps the System.getProperty call with validateApiBaseUrl, which parses the URL via java.net.URI and rejects anything that is not https or whose host does not end in .browserstack.com (IllegalArgumentException on failure). INJ-003 (CWE-22 path traversal): AppAutomateClient.uploadApp validated the file extension with a suffix-only check on the supplied filePath. A symlink legit.apk -> /etc/passwd would pass that check and stream the target's bytes to the upload endpoint. uploadApp now resolves the file via File.getCanonicalFile() (IOException is wrapped as AppAutomateException), checks the extension on the canonical file name, and uses the canonical File for the upload. New unit tests in src/test/java/com/browserstack/automate/ApiBaseUrlAndUploadSecurityTest.java cover both fixes, including a real symlink case for INJ-003 (Assume-skipped on platforms that disallow symlink creation). Resolves: APS-19024
1 parent b89196e commit 159c132

4 files changed

Lines changed: 353 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,25 @@ All notable changes to this project will be documented in this file.
2727
produce a malformed `Authorization: Basic` header. The condition is now
2828
`username == null || accessKey == null`. (APS-19019)
2929

30+
31+
- **SSRF guard on JVM-property base URL override** — both `AutomateClient` and
32+
`AppAutomateClient` constructors now validate the result of
33+
`System.getProperty("browserstack.automate.api", ...)` /
34+
`System.getProperty("browserstack.app-automate.api", ...)` before passing it
35+
to the parent constructor. Only `https` URLs whose host (parsed via
36+
`java.net.URI`) ends in `.browserstack.com` are accepted; all other values
37+
throw `IllegalArgumentException`. This prevents an attacker who can set JVM
38+
properties from redirecting every signed API request — and the Basic Auth
39+
credentials those requests carry — to an attacker-controlled host. (APS-19024)
40+
41+
- **Path canonicalization in `AppAutomateClient.uploadApp`** — the file path is
42+
now resolved via `File.getCanonicalFile()` before applying the `.apk` /
43+
`.ipa` extension check, and the canonical name (not the original input) is
44+
what the extension check sees. This prevents a symlink such as
45+
`/tmp/legit.apk -> /etc/passwd` from passing the suffix-only check and
46+
causing an arbitrary local file to be streamed to the upload endpoint.
47+
(APS-19024)
48+
3049
### CI / Release
3150

3251
- **GitHub Actions hardening**`.github/workflows/ci.yml` actions are now

src/main/java/com/browserstack/appautomate/AppAutomateClient.java

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import java.io.File;
44
import java.io.FileNotFoundException;
5+
import java.io.IOException;
6+
import java.net.URI;
57
import java.util.List;
68
import com.browserstack.automate.Automate.BuildStatus;
79
import com.browserstack.automate.exception.AppAutomateException;
@@ -32,7 +34,8 @@ public class AppAutomateClient extends BrowserStackClient implements AppAutomate
3234
* @param accessKey Browserstack accessKey
3335
*/
3436
public AppAutomateClient(String username, String accessKey) {
35-
super(System.getProperty("browserstack.app-automate.api", BASE_URL), username, accessKey);
37+
super(validateApiBaseUrl(System.getProperty("browserstack.app-automate.api", BASE_URL)),
38+
username, accessKey);
3639
}
3740

3841
/**
@@ -71,9 +74,22 @@ public AppUploadResponse uploadApp(String filePath)
7174
throw new FileNotFoundException("File not found at " + filePath);
7275
}
7376

74-
if (!filePath.endsWith(".apk") && !filePath.endsWith(".ipa")) {
77+
// APS-19024 / INJ-003: canonicalize before validating extension. Without this,
78+
// a symlink such as /tmp/legit.apk -> /etc/passwd would pass a suffix-only
79+
// check on the supplied path and result in an arbitrary local file being
80+
// streamed to the upload endpoint. getCanonicalFile() resolves symlinks
81+
// and ".." segments; we then check the extension on the canonical name.
82+
final File canonical;
83+
try {
84+
canonical = file.getCanonicalFile();
85+
} catch (IOException e) {
86+
throw new AppAutomateException("Unable to canonicalize file path: " + e.getMessage(), 0);
87+
}
88+
final String canonicalName = canonical.getName().toLowerCase();
89+
if (!canonicalName.endsWith(".apk") && !canonicalName.endsWith(".ipa")) {
7590
throw new InvalidFileExtensionException("File extension should be only .apk or .ipa.");
7691
}
92+
file = canonical;
7793

7894
MultipartContent content = new MultipartContent().setMediaType(
7995
new HttpMediaType("multipart/form-data").setParameter("boundary", "__END_OF_PART__"));
@@ -282,5 +298,46 @@ public List<Session> getSessions(final String buildId, final BuildStatus status)
282298
return getSessions(buildId, status, 0);
283299
}
284300

285-
}
286301

302+
/**
303+
* Validates the App Automate API base URL read from the JVM system property
304+
* {@code browserstack.app-automate.api} (or the built-in default).
305+
* <p>
306+
* Defends against SSRF (APS-19024 / INJ-002): an attacker who can set the
307+
* JVM property could otherwise redirect every signed API request — and the
308+
* Basic Auth credentials those requests carry — to an attacker-controlled
309+
* host. Only {@code https} URLs whose host (parsed via
310+
* {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
311+
* accepted.
312+
*
313+
* @param url URL to validate. Must be a syntactically-valid absolute URL.
314+
* @return the same URL, unchanged, when validation passes.
315+
* @throws IllegalArgumentException if the URL is null, malformed, uses a
316+
* non-https scheme, or has a host outside
317+
* {@code *.browserstack.com}.
318+
*/
319+
private static String validateApiBaseUrl(String url) {
320+
if (url == null) {
321+
throw new IllegalArgumentException("App Automate API base URL is null");
322+
}
323+
324+
final URI uri;
325+
try {
326+
uri = URI.create(url);
327+
} catch (IllegalArgumentException e) {
328+
throw new IllegalArgumentException("Malformed App Automate API base URL", e);
329+
}
330+
331+
final String scheme = uri.getScheme();
332+
if (!"https".equalsIgnoreCase(scheme)) {
333+
throw new IllegalArgumentException("Insecure App Automate API base URL scheme: " + scheme);
334+
}
335+
336+
final String host = uri.getHost();
337+
if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
338+
throw new IllegalArgumentException("Untrusted App Automate API base URL host: " + host);
339+
}
340+
341+
return url;
342+
}
343+
}

src/main/java/com/browserstack/automate/AutomateClient.java

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ public final class AutomateClient extends BrowserStackClient implements Automate
3636
* @param accessKey Access Key for your BrowserStack Automate account.
3737
*/
3838
public AutomateClient(String username, String accessKey) {
39-
super(System.getProperty("browserstack.automate.api", BASE_URL), username, accessKey);
39+
super(validateApiBaseUrl(System.getProperty("browserstack.automate.api", BASE_URL)),
40+
username, accessKey);
4041
}
4142

4243
/**
@@ -532,6 +533,48 @@ public String recycleKey() throws AutomateException {
532533
return newAccessKey;
533534
}
534535

536+
/**
537+
* Validates the API base URL read from the JVM system property
538+
* {@code browserstack.automate.api} (or the built-in default).
539+
* <p>
540+
* Defends against SSRF (APS-19024 / INJ-002): an attacker who can set the
541+
* JVM property could otherwise redirect every signed API request — and the
542+
* Basic Auth credentials those requests carry — to an attacker-controlled
543+
* host. Only {@code https} URLs whose host (parsed via
544+
* {@link java.net.URI#getHost()}) ends in {@code .browserstack.com} are
545+
* accepted.
546+
*
547+
* @param url URL to validate. Must be a syntactically-valid absolute URL.
548+
* @return the same URL, unchanged, when validation passes.
549+
* @throws IllegalArgumentException if the URL is null, malformed, uses a
550+
* non-https scheme, or has a host outside
551+
* {@code *.browserstack.com}.
552+
*/
553+
private static String validateApiBaseUrl(String url) {
554+
if (url == null) {
555+
throw new IllegalArgumentException("API base URL is null");
556+
}
557+
558+
final URI uri;
559+
try {
560+
uri = URI.create(url);
561+
} catch (IllegalArgumentException e) {
562+
throw new IllegalArgumentException("Malformed API base URL", e);
563+
}
564+
565+
final String scheme = uri.getScheme();
566+
if (!"https".equalsIgnoreCase(scheme)) {
567+
throw new IllegalArgumentException("Insecure API base URL scheme: " + scheme);
568+
}
569+
570+
final String host = uri.getHost();
571+
if (host == null || !host.toLowerCase().endsWith(".browserstack.com")) {
572+
throw new IllegalArgumentException("Untrusted API base URL host: " + host);
573+
}
574+
575+
return url;
576+
}
577+
535578
/**
536579
* Validates that a URL is safe to fetch as a BrowserStack-issued logs URL.
537580
* <p>

0 commit comments

Comments
 (0)