Merge pull request #41 from ls1intum/chore/fixes-testing-session

Improved the performance of the Wala framework and added caching mech…

Author: MarkusPaulsen

.github/workflows/maven.yml

@@ -18,7 +18,7 @@ jobs:
       - name: Checkout Repository
         uses: actions/checkout@v4
 
-      - name: Set up JDK 22
+      - name: Set up JDK 21
         uses: actions/setup-java@v4
         with:
           java-version: '21'
@@ -34,4 +34,4 @@ jobs:
         run: mvn clean package -DskipTests
 
       - name: Test
-        run: mvn test
+        run: mvn test -f pom.xml

pom.xml

@@ -4,7 +4,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>de.tum.cit.ase</groupId>
     <artifactId>ares</artifactId>
-    <version>2.0.0-Beta-2</version>
+    <version>2.0.0-Beta-4</version>
     <properties>
         <maven.compiler.source>21</maven.compiler.source>
         <arch.unit.version>1.3.0</arch.unit.version>

pom123.xml

@@ -5,6 +5,7 @@ import org.aspectj.lang.JoinPoint;
 import java.io.File;
 import java.lang.reflect.Field;
 import java.lang.reflect.InaccessibleObjectException;
+import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
 
@@ -204,7 +205,7 @@ public aspect JavaAspectJFileSystemAdviceDefinitions {
         String[] allowedPaths = (String[]) getValueFromSettings(
                 switch (action) {
                     case "read" -> "pathsAllowedToBeRead";
-                    case "write" -> "pathsAllowedToBeOverwritten";
+                    case "overwrite" -> "pathsAllowedToBeOverwritten";
                     case "execute" -> "pathsAllowedToBeExecuted";
                     case "delete" -> "pathsAllowedToBeDeleted";
                     default -> throw new IllegalArgumentException("Unknown action: " + action);
@@ -249,7 +250,7 @@ public aspect JavaAspectJFileSystemAdviceDefinitions {
                     de.tum.cit.ase.ares.api.aop.java.aspectj.adviceandpointcut.JavaAspectJFileSystemPointcutDefinitions.fileSystemProviderWriteMethods() ||
                     de.tum.cit.ase.ares.api.aop.java.aspectj.adviceandpointcut.JavaAspectJFileSystemPointcutDefinitions.printWriterInitMethods() ||
                     de.tum.cit.ase.ares.api.aop.java.aspectj.adviceandpointcut.JavaAspectJFileSystemPointcutDefinitions.desktopExecuteMethods() {
-        checkFileSystemInteraction("write", thisJoinPoint);
+        checkFileSystemInteraction("overwrite", thisJoinPoint);
     }
 
     before():

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

@@ -7,6 +7,7 @@ public aspect JavaAspectJFileSystemPointcutDefinitions {
     pointcut fileReadMethods():
             (call(* java.io.File.canRead(..)) ||
                     call(java.io.File.new(..)) ||
+                    call(java.io.RandomAccessFile.new(..)) ||
                     call(* java.io.File.exists(..)) ||
                     call(* java.io.File.getFreeSpace(..)) ||
                     call(* java.io.File.getTotalSpace(..)) ||
@@ -21,6 +22,7 @@ public aspect JavaAspectJFileSystemPointcutDefinitions {
 
     pointcut fileWriteMethods():
             (call(* java.io.File.canWrite(..)) ||
+                    call(java.io.RandomAccessFile.new(..)) ||
                     call(* java.io.File.createNewFile(..)) ||
                     call(* java.io.File.createTempFile(..)) ||
                     call(* java.io.File.setExecutable(..)) ||

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

@@ -5,8 +5,10 @@
 import java.lang.reflect.InaccessibleObjectException;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
+import java.util.List;
 
 /**
  * Utility class for the Java instrumentation advice.
@@ -20,6 +22,9 @@
  * enforce security policies at runtime and block unauthorized file interactions.
  */
 public class JavaInstrumentationAdviceToolbox {
+    private static List<String> fileSystemIgnoreAttributes = List.of("java.io.File.delete");
+    private static List<String> fileSystemIgnoreParameter = List.of();
+
     //<editor-fold desc="Constructor">
 
     /**
@@ -144,7 +149,11 @@ private static Path variableToPath(Object variableValue) {
             }
         } else if (variableValue instanceof String string) {
             try {
-                return Path.of(string).normalize().toAbsolutePath();
+                if (Files.exists(Path.of(string).normalize().toAbsolutePath())) {
+                    return Path.of(string).normalize().toAbsolutePath();
+                } else {
+                    throw new InvalidPathException(string, localize("security.advice.transform.path.exception"));
+                }
             } catch (InvalidPathException e) {
                 throw new InvalidPathException(string, localize("security.advice.transform.path.exception"));
             }
@@ -242,8 +251,11 @@ public static void checkFileSystemInteraction(
         final String fullMethodSignature = declaringTypeName + "." + methodName + methodSignature;
         String illegallyReadingMethod = allowedPaths == null ? null : checkIfCallstackCriteriaIsViolated(restrictedPackage, allowedClasses);
         if (illegallyReadingMethod != null) {
-            String illegallyReadPath = (parameters == null || parameters.length == 0) ? null : checkIfVariableCriteriaIsViolated(parameters, allowedPaths);
-            if (illegallyReadPath == null) {
+            String illegallyReadPath = null;
+            if (!fileSystemIgnoreParameter.contains(fullMethodSignature + "." + methodName)) {
+                illegallyReadPath = (parameters == null || parameters.length == 0) ? null : checkIfVariableCriteriaIsViolated(parameters, allowedPaths);
+            }
+            if (illegallyReadPath == null && !fileSystemIgnoreAttributes.contains(fullMethodSignature + "." + methodName)) {
                 illegallyReadPath = (attributes == null || attributes.length == 0) ? null : checkIfVariableCriteriaIsViolated(attributes, allowedPaths);
             }
             if (illegallyReadPath != null) {
@@ -264,7 +276,8 @@ public static String localize(String key, Object... args) {
             } else {
                 throw new IllegalStateException("Method does not return a String");
             }
-        } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
+        } catch (ClassNotFoundException | NoSuchMethodException | InvocationTargetException |
+                 IllegalAccessException e) {
             // Fallback: Return the key if localization fails
             return key;
         }

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

@@ -6,6 +6,7 @@
 import net.bytebuddy.matcher.ElementMatcher;
 import net.bytebuddy.matcher.ElementMatchers;
 
+import java.util.AbstractMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -122,29 +123,27 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      * This map contains the methods which can read files. The map keys represent class names,
      * and the values are lists of method names that are considered to be file read operations.
      */
-    public static final Map<String, List<String>> methodsWhichCanReadFiles = Map.of(
-            // TODO Markus: Currently the methods which can read are not properly instrumented
-            "java.io.FileInputStream",
-            List.of("<init>", "read", "open"),
-            "java.io.RandomAccessFile",
-            List.of("<init>", "read", "readFully", "readLine", "readBoolean", "readByte", "readChar", "readDouble",
-                    "readFloat", "readInt", "readLong", "readShort", "readUnsignedByte", "readUnsignedShort"),
-            "java.io.UnixFileSystem",
-            List.of("getBooleanAttributes0", "getSpace", "canonicalize0"),
-            "java.io.WinNTFileSystem",
-            List.of("getBooleanAttributes", "canonicalize", "getLastModifiedTime", "getSpace"),
-            "java.io.Win32FileSystem",
-            List.of("getBooleanAttributes", "canonicalize", "getLastModifiedTime", "getSpace"),
-            "java.nio.file.Files",
-            List.of("readAttributes", "readAllBytes", "readAllLines", "readString", "read", "newInputStream", "lines"),
-            "java.io.FileReader",
-            List.of("<init>", "read", "readLine"),
-            "java.io.BufferedReader",
-            List.of("lines"),
-            "java.nio.channels.FileChannel",
-            List.of("open"),
-            "java.nio.file.spi.FileSystemProvider",
-            List.of("newFileChannel")
+    public static final Map<String, List<String>> methodsWhichCanReadFiles = Map.ofEntries(
+            new AbstractMap.SimpleEntry<>("java.io.FileInputStream",
+                    List.of("<init>", "read", "open")),
+            new AbstractMap.SimpleEntry<>("java.io.RandomAccessFile",
+                    List.of("<init>")),
+            new AbstractMap.SimpleEntry<>("java.io.UnixFileSystem",
+                    List.of("getBooleanAttributes0", "getSpace", "canonicalize0")),
+            new AbstractMap.SimpleEntry<>("java.io.WinNTFileSystem",
+                    List.of("getBooleanAttributes", "canonicalize", "getLastModifiedTime", "getSpace")),
+            new AbstractMap.SimpleEntry<>("java.io.Win32FileSystem",
+                    List.of("getBooleanAttributes", "canonicalize", "getLastModifiedTime", "getSpace")),
+            new AbstractMap.SimpleEntry<>("java.nio.file.Files",
+                    List.of("readAttributes", "readAllBytes", "readAllLines", "readString", "read", "newInputStream", "lines")),
+            new AbstractMap.SimpleEntry<>("java.io.FileReader",
+                    List.of("<init>", "read", "readLine")),
+            new AbstractMap.SimpleEntry<>("java.io.BufferedReader",
+                    List.of("lines")),
+            new AbstractMap.SimpleEntry<>("sun.nio.ch.FileChannelImpl",
+                    List.of("open", "read", "readFully", "readDirect", "readIntoNativeBuffer")),
+            new AbstractMap.SimpleEntry<>("java.nio.file.spi.FileSystemProvider",
+                    List.of("newFileChannel"))
     );
     //</editor-fold>
 
@@ -153,24 +152,26 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      * This map contains the methods which can overwrite files. The map keys represent class names,
      * and the values are lists of method names that are considered to be file overwrite operations.
      */
-    public static final Map<String, List<String>> methodsWhichCanOverwriteFiles = Map.of(
-            "java.io.FileOutputStream",
-            List.of("<init>"),
-            "java.io.RandomAccessFile",
-            List.of("write", "writeBoolean", "writeByte", "writeBytes", "writeChar", "writeChars", "writeDouble", "writeFloat", "writeInt", "writeLong", "writeShort"),
-            "java.io.UnixFileSystem",
-            List.of("setLastModifiedTime", "createFileExclusively", "delete0", "createDirectory"),
-            "java.io.WinNTFileSystem",
-            List.of("createFileExclusively", "delete", "setLastModifiedTime", "createDirectory"),
-            "java.io.Win32FileSystem",
-            List.of("createFileExclusively", "delete", "setLastModifiedTime", "createDirectory"),
-            "java.util.prefs.FileSystemPreferences",
-            List.of("lockFile0", "unlockFile0"),
-            "java.nio.file.Files",
-            List.of("write", "writeString", "newOutputStream", "writeBytes", "writeAllBytes", "writeLines"),
-            "java.io.File",
-            List.of("setWritable")
-
+    public static final Map<String, List<String>> methodsWhichCanOverwriteFiles = Map.ofEntries(
+            new AbstractMap.SimpleEntry<>("java.io.FileOutputStream",
+                    List.of("<init>")),
+            new AbstractMap.SimpleEntry<>("java.io.RandomAccessFile",
+                    List.of("write", "writeBoolean", "writeByte", "writeBytes", "writeChar", "writeChars",
+                            "writeDouble", "writeFloat", "writeInt", "writeLong", "writeShort")),
+            new AbstractMap.SimpleEntry<>("java.io.UnixFileSystem",
+                    List.of("setLastModifiedTime", "createFileExclusively", "delete0", "createDirectory")),
+            new AbstractMap.SimpleEntry<>("java.io.WinNTFileSystem",
+                    List.of("createFileExclusively", "delete", "setLastModifiedTime", "createDirectory")),
+            new AbstractMap.SimpleEntry<>("java.io.Win32FileSystem",
+                    List.of("createFileExclusively", "delete", "setLastModifiedTime", "createDirectory")),
+            new AbstractMap.SimpleEntry<>("java.util.prefs.FileSystemPreferences",
+                    List.of("lockFile0", "unlockFile0")),
+            new AbstractMap.SimpleEntry<>("java.nio.file.Files",
+                    List.of("write", "writeString", "newOutputStream", "writeBytes", "writeAllBytes", "writeLines")),
+            new AbstractMap.SimpleEntry<>("java.io.File",
+                    List.of("setWritable")),
+            new AbstractMap.SimpleEntry<>("sun.nio.ch.FileChannelImpl",
+                    List.of("write", "writeFully", "writeDirect", "writeFromNativeBuffer"))
     );
     //</editor-fold>
 
@@ -200,11 +201,19 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      */
     public static final Map<String, List<String>> methodsWhichCanDeleteFiles = Map.of(
             "java.io.File",
-            List.of("delete", "deleteOnExit"),
+            List.of("deleteOnExit"),
             "java.nio.file.Files",
             List.of("delete", "deleteIfExists"),
             "sun.nio.fs.UnixFileSystemProvider",
-            List.of("implDelete")
+            List.of("implDelete"),
+            "sun.nio.fs.WindowsFileSystemProvider",
+            List.of("implDelete"),
+            "java.io.UnixFileSystem",
+            List.of("delete"),
+            "java.io.WinNTFileSystem",
+            List.of("delete"),
+            "java.io.Win32FileSystem",
+            List.of("delete")
     );
     //</editor-fold>
 

src/main/java/de/tum/cit/ase/ares/api/aop/java/instrumentation/pointcut/JavaInstrumentationPointcutDefinitions.java

@@ -18,6 +18,8 @@
 import com.tngtech.archunit.core.importer.ClassFileImporter;
 import de.tum.cit.ase.ares.api.architecture.java.wala.ReachabilityChecker;
 import de.tum.cit.ase.ares.api.util.FileTools;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.util.*;
@@ -29,51 +31,46 @@
 /**
  * Utility class to build a call graph from a class path.
  */
-public class CallGraphBuilderUtils {
+public class CustomCallgraphBuilder {
 
-    private CallGraphBuilderUtils() {
-        throw new SecurityException(localize("security.general.utility.initialization", CallGraphBuilderUtils.class.getName()));
-    }
+    private static final Logger log = LoggerFactory.getLogger(CustomCallgraphBuilder.class);
 
     /**
      * Class file importer to import the class files.
      * This is used to import the class files from the URL.
      */
-    private static final ClassFileImporter classFileImporter;
+    private final ClassFileImporter classFileImporter;
 
-    private static final ClassHierarchy classHierarchy;
+    private final ClassHierarchy classHierarchy;
 
-    private static final AnalysisScope scope;
+    private final AnalysisScope scope;
 
-    static {
-        try {
-            // Create a class file importer
-            classFileImporter = new ClassFileImporter();
+    private CallGraph callGraph = null;
 
-            // Create an analysis scope
+    public CustomCallgraphBuilder() {
+        classFileImporter = new ClassFileImporter();
+        try {
             scope = Java9AnalysisScopeReader.instance.makeJavaBinaryAnalysisScope(
                     System.getProperty("java.class.path"),
-                    // File translates the path name for Windows and Unix
                     FileTools.getResourceAsFile("de/tum/cit/ase/ares/api/templates/architecture/java/exclusions.txt")
             );
-
-            // Build the class hierarchy
             classHierarchy = ClassHierarchyFactory.make(scope);
         } catch (ClassHierarchyException | IOException e) {
-            throw new SecurityException(localize("security.architecture.class.hierarchy.error")); // $NON-NLS-1$
+            throw new SecurityException(localize("security.architecture.class.hierarchy.error"));
         }
     }
 
 
     /**
      * Try to resolve the class by the given type name.
-     *
+     * <p>
      * Ignore jrt URLs as they cause infinite loops and are not needed for the analysis for ArchUnit
      *
      * @param typeName The type name of the class to resolve.
      * @return The resolved class if it exists.
      */
-    public static Optional<JavaClass> tryResolve(String typeName) {
+    @SuppressWarnings("unused")
+    public Optional<JavaClass> tryResolve(String typeName) {
         List<String> ignoredTypeNames = List.of(
                 // Advice definition uses Reflection and therefor should not be resolved
                 "de.tum.cit.ase.ares.api.aop.java.aspectj.adviceandpointcut.JavaAspectJFileSystemAdviceDefinitions"
@@ -83,7 +80,7 @@ public static Optional<JavaClass> tryResolve(String typeName) {
             return Optional.empty();
         }
         // TODO: Check if FileTools supports this approach
-        return Optional.ofNullable(CallGraphBuilderUtils.class.getResource("/" + typeName.replace(".", "/") + ".class"))
+        return Optional.ofNullable(CustomCallgraphBuilder.class.getResource("/" + typeName.replace(".", "/") + ".class"))
                 .map(location -> classFileImporter
                         .withImportOption(loc -> !loc.contains("jrt"))
                         .importUrl(location))
@@ -102,7 +99,8 @@ public static Optional<JavaClass> tryResolve(String typeName) {
      * @param typeName The type name of the class to get the immediate subclasses.
      * @return The immediate subclasses of the given type name.
      */
-    public static Set<JavaClass> getImmediateSubclasses(String typeName) {
+    @SuppressWarnings("unused")
+    public Set<JavaClass> getImmediateSubclasses(String typeName) {
         TypeReference reference = TypeReference.find(ClassLoaderReference.Application, convertTypeName(typeName));
         if (reference == null) {
             return Collections.emptySet();
@@ -116,7 +114,7 @@ public static Set<JavaClass> getImmediateSubclasses(String typeName) {
                 .stream()
                 .map(IClass::getName)
                 .map(Object::toString)
-                .map(CallGraphBuilderUtils::tryResolve)
+                .map(this::tryResolve)
                 .filter(Optional::isPresent)
                 .map(Optional::get).collect(Collectors.toSet());
     }
@@ -137,8 +135,11 @@ public static String convertTypeName(String typeName) {
     /**
      * Build a call graph from a class path and the passed in predicate
      */
-    public static CallGraph buildCallGraph(String classPathToAnalyze) {
+    public CallGraph buildCallGraph(String classPathToAnalyze) {
         try {
+            if (callGraph != null) {
+                return callGraph;
+            }
             // Create a list to store entry points
             List<DefaultEntrypoint> customEntryPoints = ReachabilityChecker.getEntryPointsFromStudentSubmission(classPathToAnalyze, classHierarchy);
 
@@ -149,9 +150,17 @@ public static CallGraph buildCallGraph(String classPathToAnalyze) {
             CallGraphBuilder<InstanceKey> builder = Util.makeZeroOneCFABuilder(Language.JAVA, options, new AnalysisCacheImpl(), classHierarchy);
 
             // Generate the call graph
-            return builder.makeCallGraph(options, null);
+            callGraph = builder.makeCallGraph(options, null);
+            return callGraph;
         } catch (CallGraphBuilderCancelException e) {
             throw new SecurityException(localize("security.architecture.build.call.graph.error")); //$NON-NLS-1$
         }
     }
+
+    /**
+     * Get the call graph
+     */
+    public CallGraph getCallGraph() {
+        return callGraph;
+    }
 }