Skip to content

Commit fe8b908

Browse files
LukaPetrovicTUMLukaPetrovicTUM
committed
feat(aop): standard-allow low-risk entropy-device reads and default temp-file creation in the secure baseline
1 parent 644140b commit fe8b908

5 files changed

Lines changed: 146 additions & 10 deletions

File tree

‎src/main/java/de/tum/cit/ase/ares/api/aop/java/aspectj/adviceandpointcut/JavaAspectJFileSystemAdviceDefinitions.aj‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -904,18 +904,35 @@ public aspect JavaAspectJFileSystemAdviceDefinitions extends JavaAspectJAbstract
904904
}
905905
}
906906
if (explicitDirectory == null) {
907-
// No explicit directory: the JVM writes to java.io.tmpdir, a JVM/library
908-
// default, not a location the student chose.
909-
return true;
907+
// No explicit directory: the JVM writes to java.io.tmpdir - but ONLY when that
908+
// property still resolves to the value captured at class-initialisation time,
909+
// before student code could run. Student code can call
910+
// System.setProperty("java.io.tmpdir", ...) at any time (setting a property is
911+
// not itself a file operation), and the JDK's own
912+
// TempFileHelper/File$TempDirectory caches whichever value is current at THEIR
913+
// own first use - which may happen later than ours and thus already be
914+
// student-controlled. If the property no longer matches the trusted snapshot,
915+
// we cannot prove where the JDK will actually write, so this fails closed
916+
// rather than trusting the "default directory" label.
917+
if (isCurrentDefaultTempDirStillTrusted()) {
918+
return true;
919+
}
920+
throw new SecurityException(localize("security.advice.file.system.temp.directory.property.tampered",
921+
systemMethodToCheck, fullMethodSignature));
910922
}
911923
@Nullable
912924
String[] allowedPaths = getValueFromSettings("pathsAllowedToBeCreated");
913925
boolean noAllowRuleConfigured = allowedPaths == null || allowedPaths.length == 0;
914926
@Nullable
915927
String violation = checkIfVariableCriteriaIsViolated(new Object[] { explicitDirectory }, allowedPaths,
916928
IgnoreValues.NONE, true);
917-
if (violation == null || isPathWithin(violation, TRUSTED_DEFAULT_TEMP_DIR)
918-
|| INTERNAL_PATH_SUFFIXES.stream().anyMatch(violation::endsWith)) {
929+
// Deliberately no INTERNAL_PATH_SUFFIXES exemption here (unlike the generic
930+
// read/write paths below): that exemption exists for Ares's own fixed,
931+
// hardcoded classpath-resource suffixes, not for a student-supplied directory.
932+
// A student can create and name their own directory tree, so a suffix-only
933+
// match would let them craft a path ending in one of those exact strings and
934+
// bypass pathsAllowedToBeCreated entirely.
935+
if (violation == null || isPathWithin(violation, TRUSTED_DEFAULT_TEMP_DIR)) {
919936
return true;
920937
}
921938
throw new SecurityException(localize("security.advice.illegal.file.execution", systemMethodToCheck, "create",
@@ -924,6 +941,32 @@ public aspect JavaAspectJFileSystemAdviceDefinitions extends JavaAspectJAbstract
924941
+ buildDenialReason(noAllowRuleConfigured)));
925942
}
926943

944+
/**
945+
* Returns {@code true} when {@code java.io.tmpdir} still resolves to the same
946+
* location as {@link #TRUSTED_DEFAULT_TEMP_DIR}, the value captured at
947+
* class-initialisation time before student code could run. Comparison is
948+
* purely lexical (no filesystem access, matching {@link #isPathWithin}) to
949+
* avoid re-entering the interceptors.
950+
*
951+
* @return {@code true} if the property has not been redirected since startup
952+
*/
953+
private static boolean isCurrentDefaultTempDirStillTrusted() {
954+
if (TRUSTED_DEFAULT_TEMP_DIR == null) {
955+
return false;
956+
}
957+
String currentTempDir = System.getProperty("java.io.tmpdir");
958+
if (currentTempDir == null) {
959+
return false;
960+
}
961+
try {
962+
Path trusted = Path.of(TRUSTED_DEFAULT_TEMP_DIR).toAbsolutePath().normalize();
963+
Path current = Path.of(currentTempDir).toAbsolutePath().normalize();
964+
return trusted.equals(current);
965+
} catch (InvalidPathException ignored) {
966+
return false;
967+
}
968+
}
969+
927970
// </editor-fold>
928971

929972
/**

‎src/main/java/de/tum/cit/ase/ares/api/aop/java/instrumentation/advice/JavaInstrumentationAdviceFileSystemToolbox.java‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -888,18 +888,35 @@ private static boolean checkTempFileCreationSpecialCase(@Nonnull String action,
888888
}
889889
}
890890
if (explicitDirectory == null) {
891-
// No explicit directory: the JVM writes to java.io.tmpdir, a JVM/library
892-
// default, not a location the student chose.
893-
return true;
891+
// No explicit directory: the JVM writes to java.io.tmpdir - but ONLY when that
892+
// property still resolves to the value captured at class-initialisation time,
893+
// before student code could run. Student code can call
894+
// System.setProperty("java.io.tmpdir", ...) at any time (setting a property is
895+
// not itself a file operation), and the JDK's own
896+
// TempFileHelper/File$TempDirectory caches whichever value is current at THEIR
897+
// own first use - which may happen later than ours and thus already be
898+
// student-controlled. If the property no longer matches the trusted snapshot,
899+
// we cannot prove where the JDK will actually write, so this fails closed
900+
// rather than trusting the "default directory" label.
901+
if (isCurrentDefaultTempDirStillTrusted()) {
902+
return true;
903+
}
904+
throw new SecurityException(localize("security.advice.file.system.temp.directory.property.tampered",
905+
fileSystemMethodToCheck, fullMethodSignature));
894906
}
895907
@Nullable
896908
String[] allowedPaths = getValueFromSettings("pathsAllowedToBeCreated");
897909
boolean noAllowRuleConfigured = allowedPaths == null || allowedPaths.length == 0;
898910
@Nullable
899911
String violation = checkIfVariableCriteriaIsViolated(new Object[] { explicitDirectory }, allowedPaths,
900912
IgnoreValues.NONE, true);
901-
if (violation == null || isPathWithin(violation, TRUSTED_DEFAULT_TEMP_DIR)
902-
|| INTERNAL_PATH_SUFFIXES.stream().anyMatch(violation::endsWith)) {
913+
// Deliberately no INTERNAL_PATH_SUFFIXES exemption here (unlike the generic
914+
// read/write paths below): that exemption exists for Ares's own fixed,
915+
// hardcoded classpath-resource suffixes, not for a student-supplied directory.
916+
// A student can create and name their own directory tree, so a suffix-only
917+
// match would let them craft a path ending in one of those exact strings and
918+
// bypass pathsAllowedToBeCreated entirely.
919+
if (violation == null || isPathWithin(violation, TRUSTED_DEFAULT_TEMP_DIR)) {
903920
return true;
904921
}
905922
throw new SecurityException(
@@ -909,6 +926,32 @@ private static boolean checkTempFileCreationSpecialCase(@Nonnull String action,
909926
+ " | " + buildDenialReason(noAllowRuleConfigured)));
910927
}
911928

929+
/**
930+
* Returns {@code true} when {@code java.io.tmpdir} still resolves to the same
931+
* location as {@link #TRUSTED_DEFAULT_TEMP_DIR}, the value captured at
932+
* class-initialisation time before student code could run. Comparison is purely
933+
* lexical (no filesystem access, matching {@link #isPathWithin}) to avoid
934+
* re-entering the interceptors.
935+
*
936+
* @return {@code true} if the property has not been redirected since startup
937+
*/
938+
private static boolean isCurrentDefaultTempDirStillTrusted() {
939+
if (TRUSTED_DEFAULT_TEMP_DIR == null) {
940+
return false;
941+
}
942+
String currentTempDir = System.getProperty("java.io.tmpdir");
943+
if (currentTempDir == null) {
944+
return false;
945+
}
946+
try {
947+
Path trusted = Path.of(TRUSTED_DEFAULT_TEMP_DIR).toAbsolutePath().normalize();
948+
Path current = Path.of(currentTempDir).toAbsolutePath().normalize();
949+
return trusted.equals(current);
950+
} catch (InvalidPathException ignored) {
951+
return false;
952+
}
953+
}
954+
912955
/**
913956
* Checks a single isolated parameter/receiver candidate array against one
914957
* specific allow-list, throwing {@code SecurityException} on violation. Applies

‎src/main/resources/de/tum/cit/ase/ares/api/localization/messages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ security.advice.thread.task.null=Ares Security Error (Reason: Ares-Code; Stage:
6666
security.advice.thread.task.reflection.error=Ares Security Error (Reason: Ares-Code; Stage: Execution): Failed to inspect thread task for field '%s' due to reflection access failure.
6767
security.advice.file.system.unknown.action=Ares Security Error (Reason: Ares-Code; Stage: Execution): Unknown file system action '%s'.
6868
security.advice.file.system.malformed.temp.file.creation=Ares Security Error (Reason: Student-Code; Stage: Execution): %s presented an unresolved or wrongly shaped directory argument via %s and was denied as a fail-closed precaution.
69+
security.advice.file.system.temp.directory.property.tampered=Ares Security Error (Reason: Student-Code; Stage: Execution): %s could not verify that java.io.tmpdir still points to the trusted default temp directory captured at startup, so the implicit-directory call via %s was denied as a fail-closed precaution.
6970
security.advice.thread.allowed.size=Ares Security Error (Reason: Ares-Code; Stage: Execution): The number of elements in threadNumberAllowedToBeCreated (currently '%s') must be equal to the number of elements in threadClassAllowedToBeCreated (currently '%s').
7071
security.advice.command.allowed.size=Ares Security Error (Reason: Ares-Code; Stage: Execution): The number of elements in argumentsAllowedToBePassed (currently '%s') must be equal to the number of elements in commandsAllowedToBeExecuted (currently '%s').
7172
security.advice.command.pipeline.element.invalid=Ares Security Error (Reason: Student-Code; Stage: Execution): ProcessBuilder.startPipeline was called with a pipeline element that is not a ProcessBuilder and was blocked by Ares.

‎src/main/resources/de/tum/cit/ase/ares/api/localization/messages_de.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ security.advice.thread.task.null=Ares-Sicherheitsfehler (Grund: Ares-Code; Phase
7373
security.advice.thread.task.reflection.error=Ares-Sicherheitsfehler (Grund: Ares-Code; Phase: Ausf\u0308hrung): Der Thread-Task f\u0308r das Feld '%s' konnte aufgrund eines Reflexionsfehlers nicht ausgelesen werden.
7474
security.advice.file.system.unknown.action=Ares Sicherheitsfehler (Grund: Ares-Code; Phase: Ausf\u0308hrung): Unbekannte Dateisystemaktion '%s'.
7575
security.advice.file.system.malformed.temp.file.creation=Ares Sicherheitsfehler (Grund: Student-Code; Phase: Ausf\u00fchrung): %s hat ein nicht aufl\u00f6sbares oder falsch geformtes Verzeichnisargument \u00fcber %s \u00fcbergeben und wurde als Fail-Closed-Vorsichtsma\u00dfnahme abgelehnt.
76+
security.advice.file.system.temp.directory.property.tampered=Ares Sicherheitsfehler (Grund: Student-Code; Phase: Ausf\u00fchrung): %s konnte nicht best\u00e4tigen, dass java.io.tmpdir weiterhin auf das beim Start erfasste vertrauensw\u00fcrdige Standardverzeichnis f\u00fcr tempor\u00e4re Dateien verweist, weshalb der Aufruf ohne explizites Verzeichnis \u00fcber %s als Fail-Closed-Vorsichtsma\u00dfnahme abgelehnt wurde.
7677
security.advice.thread.allowed.size=Ares Sicherheitsfehler (Grund: Ares-Code; Phase: Ausf\u0308hrung): Die Anzahl der Elemente in threadNumberAllowedToBeCreated (derzeit '%s') muss gleich der Anzahl der Elemente in threadClassAllowedToBeCreated (derzeit '%s') sein.
7778
security.instrumentation.inaccessible.object.exception=Ares Sicherheitsfehler (Grund: Ares-Code; Phase: Ausf\u0308hrung): Das Feld '%s' in der Klasse '%s' kann aufgrund von JVM-Sicherheitsbeschr�nkungen nicht zug�nglich gemacht werden.
7879
security.instrumentation.illegal.access.exception=Ares Sicherheitsfehler (Grund: Ares-Code; Phase: Ausf\u0308hrung): Der Zugriff auf das Feld '%s' in der Klasse '%s' wurde verweigert. Der Zugriff auf das Feld ist nicht gestattet.

‎src/test/java/de/tum/cit/ase/ares/api/aop/java/instrumentation/advice/JavaInstrumentationAdviceFileSystemToolboxTest.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,54 @@ void filesCreateTempFileWithExplicitNonAllowedDirectoryIsDenied() throws Excepti
694694
}
695695
}
696696

697+
@Test
698+
void explicitTempDirectoryEndingInInternalPathSuffixIsNoLongerExemptFromAllowlist() throws Exception {
699+
try {
700+
resetSettings();
701+
configureInstrumentationMode();
702+
JavaAOPTestCase.setJavaAdviceSettingValue("pathsAllowedToBeCreated", new String[0], "ARCH",
703+
"INSTRUMENTATION");
704+
705+
// Regression guard: INTERNAL_PATH_SUFFIXES exists to exempt Ares's own fixed,
706+
// hardcoded classpath-resource reads (e.g. its localization bundle), not
707+
// student-supplied create directories. A student who names their own
708+
// directory tree to end with one of those exact suffix strings must still
709+
// be denied, not silently treated as "internal Ares file access".
710+
File explicitDir = Path.of("target", "baseline-low-risk-test-dirs", "student-crafted", "ares", "api",
711+
"localization", "Messages.class").toFile();
712+
assertThrows(SecurityException.class,
713+
() -> InstrumentationSecurityProbe.checkFileCreateTempFile("ares-baseline-", ".tmp", explicitDir));
714+
} finally {
715+
resetSettings();
716+
}
717+
}
718+
719+
@Test
720+
void javaIoTmpdirRedirectionAfterStartupIsDeniedForImplicitDirectoryCreation() throws Exception {
721+
String originalTmpDir = System.getProperty("java.io.tmpdir");
722+
try {
723+
resetSettings();
724+
configureInstrumentationMode();
725+
JavaAOPTestCase.setJavaAdviceSettingValue("pathsAllowedToBeCreated", new String[0], "ARCH",
726+
"INSTRUMENTATION");
727+
728+
// Regression guard: java.io.tmpdir is mutable at runtime via
729+
// System.setProperty, while TRUSTED_DEFAULT_TEMP_DIR is captured once, at
730+
// class-initialisation time, before student code could run. If a later
731+
// mutation weren't detected, the no-directory overloads would stay
732+
// unconditionally exempt even though the JDK's own temp-directory helpers
733+
// may end up using the redirected (student-controlled) location instead.
734+
System.setProperty("java.io.tmpdir",
735+
createNonTempDirOutsideDefaultTempDir("javaIoTmpdirRedirection").getAbsolutePath());
736+
737+
assertThrows(SecurityException.class,
738+
() -> InstrumentationSecurityProbe.checkFilesCreateTempFile(null, "ares-baseline-", ".tmp"));
739+
} finally {
740+
System.setProperty("java.io.tmpdir", originalTmpDir);
741+
resetSettings();
742+
}
743+
}
744+
697745
@Test
698746
void filesCreateTempFileWithWrongParameterCountFailsClosed() throws Exception {
699747
try {

0 commit comments

Comments
 (0)