Skip to content

Commit

Permalink
Merge pull request from GHSA-59g4-hpg3-3gcp
Browse files Browse the repository at this point in the history
Ensure local files are not addable
  • Loading branch information
gregorydlogan committed Dec 13, 2021
2 parents 776d558 + d881ba6 commit 65c46b9
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,8 @@ public class IngestServiceImpl extends AbstractJobProducer implements IngestServ
private boolean skipCatalogs = DEFAULT_SKIP;
private boolean skipAttachments = DEFAULT_SKIP;

protected boolean testMode = false;

/**
* Creates a new ingest service instance.
*/
Expand Down Expand Up @@ -1601,8 +1603,11 @@ protected URI addContentToRepo(MediaPackage mp, String elementId, URI uri) throw
throw new IOException(uri + " returns http " + httpStatusCode);
}
in = response.getEntity().getContent();
} else {
//If it does not start with file, or we're in test mode (ie, to allow arbitrary file:// access)
} else if (!uri.toString().startsWith("file") || testMode) {
in = uri.toURL().openStream();
} else {
throw new IOException("Refusing to fetch files from the local filesystem");
}
String fileName = FilenameUtils.getName(uri.getPath());
if (isBlank(FilenameUtils.getExtension(fileName)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ protected CloseableHttpClient getNoAuthHttpClient() {
Dictionary<String, String> p = new Hashtable<>();
p.put(IngestServiceImpl.DOWNLOAD_SOURCE, "http://localhost.*|http://www.test.com/.*");
service.updated(p);
service.testMode = true;
}

@After
Expand Down Expand Up @@ -491,6 +492,13 @@ public void testAuthWhitelist() throws Exception {
testAuthWhitelist("http://www.example.org/testfile", "http://localhost.*", true, false, true);
//Matching regex
testAuthWhitelist("http://www.example.org/testfile", "http://localhost.*|http://www.example.org/.*", false, true, true);

//Local filesystem should be actively rejected. This file needs to *not* be in the resources directory (look in the impl for why), and needs to be readable by the user running the test
//NB: This is a horrible, horrible hack, but it's the only way I can think of to get *out* of test-classes. If you try and ../ your way up above that getResource NPEs, as expected.
testAuthWhitelist(getClass().getResource("./../../../../").toURI().resolve("../../pom.xml").toString(), ".*", true, false, false);
//Test to ensure we can't use '..' to get around filters. Removing the ".." works as expected, see below
testAuthWhitelist(getClass().getResource("./../impl/IngestServiceImplTest.class").toURI().toString(), ".*", true, false, false);
testAuthWhitelist(getClass().getResource("./IngestServiceImplTest.class").toURI().toString(), ".*", false, false, false);
}


Expand Down

0 comments on commit 65c46b9

Please sign in to comment.