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 c152141
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
5 changes: 5 additions & 0 deletions pic-sure-resources/pic-sure-resource-api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@
<version>5.9.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>edu.harvard.hms.dbmi.avillach</groupId>
<artifactId>pic-sure-api-data</artifactId>
<version>2.1.0-SNAPSHOT</version>
</dependency>

</dependencies>
</project>
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package edu.harvard.dbmi.avillach.service;

import edu.harvard.dbmi.avillach.data.repository.ResourceRepository;
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 @@ -13,21 +13,29 @@
import org.slf4j.LoggerFactory;

import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;
import javax.ws.rs.core.Response;
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;

@Inject
ResourceRepository resourceRepository;

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

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

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

private boolean containerIsNOTAResource(String container) {
return resourceRepository.getByColumn("name", container).isEmpty();
}

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
@@ -1,5 +1,7 @@
package edu.harvard.dbmi.avillach.service;

import edu.harvard.dbmi.avillach.data.entity.Resource;
import edu.harvard.dbmi.avillach.data.repository.ResourceRepository;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
Expand All @@ -16,6 +18,7 @@
import javax.ws.rs.core.Response;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.util.List;

import static org.junit.Assert.*;

Expand All @@ -31,6 +34,9 @@ public class ProxyWebClientTest {
@Mock
private HttpEntity entity;

@Mock
private ResourceRepository resourceRepository;

@InjectMocks
private ProxyWebClient subject;

Expand All @@ -42,6 +48,8 @@ public void shouldPostToProxy() throws IOException {
.thenReturn(entity);
Mockito.when(entity.getContent())
.thenReturn(new ByteArrayInputStream("{}".getBytes()));
Mockito.when(resourceRepository.getByColumn("name", "foo"))
.thenReturn(List.of(new Resource()));
subject.client = client;

Response actual = subject.postProxy("foo", "/my/cool/path", "{}");
Expand All @@ -57,10 +65,24 @@ public void shouldGetToProxy() throws IOException {
.thenReturn(entity);
Mockito.when(entity.getContent())
.thenReturn(new ByteArrayInputStream("{}".getBytes()));
Mockito.when(resourceRepository.getByColumn("name", "bar"))
.thenReturn(List.of(new Resource()));
subject.client = client;

Response actual = subject.getProxy("bar", "/my/cool/path");

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

@Test
public void shouldRejectNastyHost() {
Mockito.when(resourceRepository.getByColumn("name", "an.evil.domain"))
.thenReturn(List.of());

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 c152141

Please sign in to comment.