Skip to content

Commit

Permalink
[ALS-0000] Checkmarx Fix - Check domain of proxy request
Browse files Browse the repository at this point in the history
- We don't want an attacker leveraging the proxy client
to make requests to 3rd party domains
- At the moment, this is just intended for docker containers in the
same network, so I just made a regex that restricts the host to
reasonable container names
  • Loading branch information
Luke Sikina committed Nov 20, 2023
1 parent f4fb15f commit cad2bda
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package edu.harvard.dbmi.avillach.service;

import edu.harvard.dbmi.avillach.util.HttpClientUtil;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
Expand All @@ -17,17 +16,24 @@
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.regex.Pattern;

@ApplicationScoped
public class ProxyWebClient {
private static final Logger LOG = LoggerFactory.getLogger(ProxyWebClient.class);
HttpClient client;

// containers must start with a letter and only contain letters, dashes and underscores
private static final Pattern DOCKER_NAME_REGEX = Pattern.compile("^[A-z][A-z-_]+$");

public ProxyWebClient() {
client = HttpClientUtil.getConfiguredHttpClient();
}

public Response postProxy(String containerId, String path, String body) {
if (!containerNameIsSafe(containerId)) {
return Response.status(400, "container name not trustworthy").build();
}
try {
URI uri = new URIBuilder()
.setScheme("http")
Expand All @@ -47,6 +53,9 @@ public Response postProxy(String containerId, String path, String body) {
}

public Response getProxy(String containerId, String path) {
if (!containerNameIsSafe(containerId)) {
return Response.status(400, "container name not trustworthy").build();
}
try {
URI uri = new URIBuilder()
.setScheme("http")
Expand All @@ -63,6 +72,10 @@ public Response getProxy(String containerId, String path) {
}
}

private boolean containerNameIsSafe(String container) {
return DOCKER_NAME_REGEX.matcher(container).matches();
}

private Response getResponse(HttpRequestBase request) throws IOException {
HttpResponse response = client.execute(request);
return Response.ok(response.getEntity().getContent()).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,13 @@ public void shouldGetToProxy() throws IOException {

Assert.assertEquals(200, actual.getStatus());
}

@Test
public void shouldRejectNastyHost() {
Response actual = subject.postProxy("an.evil.domain", "hax", null);
assertEquals(400, actual.getStatus());

actual = subject.getProxy("an.evil.domain", "hax");
assertEquals(400, actual.getStatus());
}
}

0 comments on commit cad2bda

Please sign in to comment.