Skip to content

Commit

Permalink
[s7Ob7Zm7] changed tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@vga91
vga91 committed May 4, 2023
1 parent f6f11ff commit 6b1e19c
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 43 deletions.
81 changes: 42 additions & 39 deletions core/src/test/java/apoc/export/ExportCoreSecurityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import apoc.export.json.ExportJson;
import apoc.util.FileUtils;
import apoc.util.TestUtil;
import apoc.util.Util;
import com.nimbusds.jose.util.Pair;
import org.junit.Assert;
import org.junit.BeforeClass;
Expand Down Expand Up @@ -140,37 +141,37 @@ public TestIllegalExternalFSAccess(String exportMethod, String exportMethodType,
* test "directoryName.startsWith" logic which is a common path traversal bug.
* All these tests should fail because they access a directory which isn't the configured directory
*/
private static final String case1 = "../imported/" + FILENAME;
private static final String case2 = "tests/../../imported/" + FILENAME;
private static final String case3 = "../" + FILENAME;
private static final String case4 = "file:../" + FILENAME;
private static final String case5 = "file:..//" + FILENAME;
private static final String case01 = "../imported/" + FILENAME;
private static final String case02 = "tests/../../imported/" + FILENAME;
private static final String case03 = "../" + FILENAME;
private static final String case04 = "file:../" + FILENAME;
private static final String case05 = "file:..//" + FILENAME;

// non-failing cases, with apoc.import.file.use_neo4j_config=false
public static final List<String> casesAllowed = Arrays.asList(case3, case4, case5);
public static final List<String> casesAllowed = Arrays.asList(case03, case04, case05);

private static final String case16 = "file:///%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2fapoc/" + FILENAME;
public static final String case26 = "file:///%2e%2e%2f%2f" + FILENAME;
private static final String case46 = "tests/../../" + FILENAME;
private static final String case56 = "tests/..//..//" + FILENAME;
private static final String case06 = "file:///%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2f%2e%2e%2f%2fapoc/" + FILENAME;
public static final String case07 = "file:///%2e%2e%2f%2f" + FILENAME;
private static final String case08 = "tests/../../" + FILENAME;
private static final String case09 = "tests/..//..//" + FILENAME;

public static final List<String> casesOutsideDir = Arrays.asList(case1, case2, case3, case4, case5,
case16, case26, case46, case56);
public static final List<String> casesOutsideDir = Arrays.asList(case01, case02, case03, case04, case05,
case06, case07, case08, case09);

/*
All of these will resolve to a local path after normalization which will point to
a non-existing directory in our import folder: /apoc. Causing them to error that is
not found. They all attempt to exit the import folder back to the apoc folder:
Directory Layout: .../apoc/core/target/import
*/
private static final String case111 = "file://%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f/apoc/" + FILENAME;
private static final String case211 = "file://../../../../apoc/" + FILENAME;
private static final String case311 = "file:///..//..//..//..//apoc//core//..//" + FILENAME;
private static final String case411 = "file:///..//..//..//..//apoc/" + FILENAME;
private static final String case511 = "file://" + directory.getAbsolutePath() + "//..//..//..//..//apoc/" + FILENAME;
private static final String case611 = "file:///%252e%252e%252f%252e%252e%252f%252e%252e%252f%252e%252e%252f/apoc/" + FILENAME;
private static final String case10 = "file://%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f/apoc/" + FILENAME;
private static final String case11 = "file://../../../../apoc/" + FILENAME;
private static final String case12 = "file:///..//..//..//..//apoc//core//..//" + FILENAME;
private static final String case13 = "file:///..//..//..//..//apoc/" + FILENAME;
private static final String case14 = "file://" + directory.getAbsolutePath() + "//..//..//..//..//apoc/" + FILENAME;
private static final String case15 = "file:///%252e%252e%252f%252e%252e%252f%252e%252e%252f%252e%252e%252f/apoc/" + FILENAME;

public static final List<String> casesNotExistingDir = Arrays.asList(case111, case211, case311, case411, case511, case611);
public static final List<String> casesNotExistingDir = Arrays.asList(case10, case11, case12, case13, case14, case15);

public static List<Pair<String, Consumer<Map>>> dataPairs;

Expand Down Expand Up @@ -224,14 +225,16 @@ public void testWithUseNeo4jConfDisabled() {

private void testWithUseNeo4jConfFalse() {
// with `apoc.import.file.use_neo4j_config=false` this file export could outside the project
if (fileName.equals(case26)) {
if (fileName.equals(case07)) {
return;
}

if (casesAllowed.contains(fileName)) {
try {
assertPathTraversalWithoutErrors();
} else {
assertPathTraversalError(db, apocProcedure, Map.of("fileName", fileName), EXCEPTION_NOT_FOUND_CONSUMER);
} catch (QueryExecutionException e) {
EXCEPTION_NOT_FOUND_CONSUMER.accept(
Util.map(ERROR_KEY, e, PROCEDURE_KEY, apocProcedure)
);
}
}

Expand Down Expand Up @@ -267,31 +270,31 @@ public TestPathTraversalIsNormalisedWithinDirectory(String exportMethod, String
These tests normalize the path to be within the import directory and make the file there.
They result in a file being created (and deleted after).
*/
private static final String case0 = "./" + FILENAME;
private static final String caseBase = "./" + FILENAME;

private static final String case1 = "file:///..//..//..//..//apoc//..//..//..//..//" + FILENAME;
private static final String case2 = "file:///..//..//..//..//apoc//..//" + FILENAME;
private static final String case3 = "file:///../import/../import//..//" + FILENAME;
private static final String case4 = "file://" + FILENAME;
private static final String case5 = "file://tests/../" + FILENAME;
private static final String case6 = "file:///tests//..//" + FILENAME;
private static final String case7 = "" + FILENAME;
private static final String case8 = "file:///..//..//..//..//" + FILENAME;
private static final String case01 = "file:///..//..//..//..//apoc//..//..//..//..//" + FILENAME;
private static final String case02 = "file:///..//..//..//..//apoc//..//" + FILENAME;
private static final String case03 = "file:///../import/../import//..//" + FILENAME;
private static final String case04 = "file://" + FILENAME;
private static final String case05 = "file://tests/../" + FILENAME;
private static final String case06 = "file:///tests//..//" + FILENAME;
private static final String case07 = "" + FILENAME;
private static final String case08 = "file:///..//..//..//..//" + FILENAME;

public static final List<String> mainDirCases = Arrays.asList(case0, case1, case2, case3, case4, case5, case6, case7, case8);
public static final List<String> mainDirCases = Arrays.asList(caseBase, case01, case02, case03, case04, case05, case06, case07, case08);

/*
These tests normalize the path to be within the import directory and step into a subdirectory
to make the file there.
They result in a file in the directory /tests being created (and deleted after).
*/
private static final String case16 = "file:///../import/../import//..//tests/" + FILENAME;
private static final String case26 = "file:///..//..//..//..//apoc//..//tests/" + FILENAME;
private static final String case36 = "file:///../import/../import//..//tests/../tests/" + FILENAME;
private static final String case46 = "file:///tests/" + FILENAME;
private static final String case56 = "tests/" + FILENAME;
private static final String case11 = "file:///../import/../import//..//tests/" + FILENAME;
private static final String case12 = "file:///..//..//..//..//apoc//..//tests/" + FILENAME;
private static final String case13 = "file:///../import/../import//..//tests/../tests/" + FILENAME;
private static final String case14 = "file:///tests/" + FILENAME;
private static final String case15 = "tests/" + FILENAME;

public static final List<String> subDirCases = Arrays.asList(case16, case26, case36, case46, case56);
public static final List<String> subDirCases = Arrays.asList(case11, case12, case13, case14, case15);


@Parameterized.Parameters(name = PARAM_NAMES)
Expand Down
7 changes: 6 additions & 1 deletion core/src/test/java/apoc/export/ImportAndLoadCoreSecurityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.io.IOException;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -295,7 +296,11 @@ private void shouldRead() {
Result::resultAsString);
} catch (Exception e) {
if (ALLOWED_EXCEPTIONS.containsKey(importMethod)) {
assertEquals(ALLOWED_EXCEPTIONS.get(importMethod), ExceptionUtils.getRootCause(e).getClass());
Class<?> rootCause = ExceptionUtils.getRootCause(e).getClass();
Class<?> classException = ALLOWED_EXCEPTIONS.get(importMethod);
assertTrue("The procedure throws an exception with class " + rootCause + " instead of " + classException,
classException.isAssignableFrom(rootCause)
);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/apoc/export/SecurityTestUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class SecurityTestUtil {

// import allowed exception
"apoc.import.json", JsonParseException.class,
"apoc.import.csv", NoSuchElementException.class,
"apoc.import.csv", RuntimeException.class,
"apoc.import.graphml", JsonParseException.class,
"apoc.import.xml", WstxUnexpectedCharException.class
);
Expand Down Expand Up @@ -74,6 +74,7 @@ public static void assertPathTraversalError(GraphDatabaseService db, String quer
() -> TestUtil.testCall(db, query, params, (r) -> {})
);


// this consumer accept a Map.of("error", <errorMessage>, "procedure", <procedureQuery>)
exceptionConsumer.accept(
Util.map(ERROR_KEY, e, PROCEDURE_KEY, query)
Expand Down
4 changes: 2 additions & 2 deletions core/src/test/java/apoc/export/arrow/ExportArrowSecurityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@

import static apoc.export.ExportCoreSecurityTest.FILENAME;
import static apoc.export.ExportCoreSecurityTest.PARAM_NAMES;
import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.case26;
import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.case07;
import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.casesAllowed;
import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.EXCEPTION_NOT_FOUND_CONSUMER;
import static apoc.export.ExportCoreSecurityTest.TestIllegalExternalFSAccess.dataPairs;
Expand Down Expand Up @@ -145,7 +145,7 @@ public void testWithUseNeo4jConfDisabled() {

private void testWithUseNeo4jConfFalse() {
// with `apoc.import.file.use_neo4j_config=false` this file export could outside the project
if (fileName.equals(case26)) {
if (fileName.equals(case07)) {
return;
}

Expand Down

0 comments on commit 6b1e19c

Please sign in to comment.