Merge pull request #30 from ls1intum/chore/fix-println-error

`AOP`: Fix errors for Instrumentation regarding Pointcuts

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

@@ -48,6 +48,23 @@ public aspect JavaAspectJFileSystemAdviceDefinitions {
     //<editor-fold desc="File system methods">
 
     //<editor-fold desc="Callstack criteria methods">
+    /**
+     * Check if the provided call stack element is allowed.
+     * This method verifies whether the class in the call stack element belongs to the list of allowed
+     * classes, ensuring that only authorized classes are permitted to perform certain file system operations.
+     *
+     * @param allowedClasses The list of classes allowed to be present in the call stack.
+     * @param elementToCheck The call stack element to check.
+     * @return True if the call stack element is allowed, false otherwise.
+     */
+    private static boolean checkIfCallstackElementIsAllowed(String[] allowedClasses, StackTraceElement elementToCheck) {
+        for (String allowedClass : allowedClasses) {
+            if (elementToCheck.getClassName().startsWith(allowedClass)) {
+                return true;
+            }
+        }
+        return false;
+    }
 
     /**
      * Check if the call stack violates the specified criteria.
@@ -61,13 +78,18 @@ public aspect JavaAspectJFileSystemAdviceDefinitions {
      * @return The call stack element that violates the criteria, or null if no violation occurred.
      */
     private static String checkIfCallstackCriteriaIsViolated(String restrictedPackage, String[] allowedClasses, String readingMethod) {
-        if (readingMethod.startsWith(restrictedPackage)) {
-            for (String allowedClass : allowedClasses) {
-                if (readingMethod.startsWith(allowedClass)) {
+        StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
+        for (StackTraceElement element : stackTrace) {
+            if (element.getClassName().startsWith(restrictedPackage)) {
+                // Skip the OutputTester and InputTester classes, as they intercept the output and input for System.out and System.in
+                // Therefore, they cause false positives.
+                if (element.toString().equals("de.tum.cit.ase.ares.api.io.OutputTester") || element.toString().equals("de.tum.cit.ase.ares.api.io.InputTester")) {
                     return null;
                 }
+                if (!checkIfCallstackElementIsAllowed(allowedClasses, element)) {
+                    return element.getClassName();
+                }
             }
-            return readingMethod;
         }
         return null;
     }

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

@@ -100,9 +100,15 @@ private static boolean checkIfCallstackElementIsAllowed(String[] allowedClasses,
     private static String checkIfCallstackCriteriaIsViolated(String restrictedPackage, String[] allowedClasses) {
         StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
         for (StackTraceElement element : stackTrace) {
-            if (element.toString().startsWith(restrictedPackage)) {
+            if (element.getClassName().startsWith(restrictedPackage)) {
+                // Skip the OutputTester and InputTester classes, as they intercept the output and input for System.out and System.in
+                // Therefore, they cause false positives.
+                // Also, X11FontManager needs to be set when using AWT therefore we have to allow it
+                if (element.getClassName().equals("de.tum.cit.ase.ares.api.io.OutputTester") || element.getClassName().equals("de.tum.cit.ase.ares.api.io.InputTester") || element.getClassName().equals("sun.awt.X11FontManager")) {
+                    return null;
+                }
                 if (!checkIfCallstackElementIsAllowed(allowedClasses, element)) {
-                    return element.toString();
+                    return element.getClassName();
                 }
             }
         }
@@ -225,7 +231,7 @@ public static void checkFileSystemInteraction(
         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);

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

@@ -121,13 +121,14 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      * 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>"),
+            List.of("<init>", "read", "open"),
             "java.io.RandomAccessFile",
-            List.of("read", "readFully", "readLine", "readBoolean", "readByte", "readChar", "readDouble",
+            List.of("<init>", "read", "readFully", "readLine", "readBoolean", "readByte", "readChar", "readDouble",
                     "readFloat", "readInt", "readLong", "readShort", "readUnsignedByte", "readUnsignedShort"),
             "java.io.UnixFileSystem",
-            List.of("getLastModifiedTime", "getBooleanAttributes0", "getSpace", "canonicalize0"),
+            List.of("getBooleanAttributes0", "getSpace", "canonicalize0"),
             "java.io.WinNTFileSystem",
             List.of("getBooleanAttributes", "canonicalize", "getLastModifiedTime", "getSpace"),
             "java.io.Win32FileSystem",
@@ -137,7 +138,9 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
             "java.io.FileReader",
             List.of("<init>", "read", "readLine"),
             "java.io.BufferedReader",
-            List.of("lines")
+            List.of("lines"),
+            "java.nio.channels.FileChannel",
+            List.of("open")
     );
     //</editor-fold>
 
@@ -158,7 +161,14 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
             "java.io.Win32FileSystem",
             List.of("createFileExclusively", "delete", "setLastModifiedTime", "createDirectory"),
             "java.util.prefs.FileSystemPreferences",
-            List.of("lockFile0", "unlockFile0")
+            List.of("lockFile0", "unlockFile0"),
+            "java.nio.file.Files",
+            List.of("write", "writeString", "newOutputStream", "writeBytes", "writeAllBytes", "writeLines"),
+            "java.io.File",
+            List.of("setWritable"),
+            "java.nio.channels.FileChannel",
+            List.of("open")
+
     );
     //</editor-fold>
 
@@ -168,12 +178,16 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      * and the values are lists of method names that are considered to be file execute operations.
      */
     public static final Map<String, List<String>> methodsWhichCanExecuteFiles = Map.of(
-            "java.io.UnixFileSystem",
-            List.of("checkAccess", "setPermission"),
+            "java.io.File",
+            List.of("setExecutable"),
             "java.io.WinNTFileSystem",
-            List.of("checkAccess", "setReadOnly"),
+            List.of("setReadOnly"),
             "java.io.Win32FileSystem",
-            List.of("checkAccess", "setReadOnly")
+            List.of("checkAccess", "setReadOnly"),
+            "java.nio.file.Files",
+            List.of("setPosixFilePermissions"),
+            "java.awt.Desktop",
+            List.of("open", "edit", "print", "browse", "mail")
     );
     //</editor-fold>
 
@@ -184,7 +198,11 @@ static ElementMatcher<MethodDescription> getMethodsMatcher(
      */
     public static final Map<String, List<String>> methodsWhichCanDeleteFiles = Map.of(
             "java.io.File",
-            List.of("delete", "deleteOnExit")
+            List.of("delete", "deleteOnExit"),
+            "java.nio.file.Files",
+            List.of("delete", "deleteIfExists"),
+            "sun.nio.fs.UnixFileSystemProvider",
+            List.of("implDelete")
     );
     //</editor-fold>
 

src/main/java/de/tum/cit/ase/ares/api/architecture/java/archunit/FileHandlerConstants.java

@@ -2,7 +2,6 @@
 
 import de.tum.cit.ase.ares.api.util.FileTools;
 
-import java.io.File;
 import java.nio.file.Path;
 
 /**
@@ -15,6 +14,7 @@ public class FileHandlerConstants {
     public static final Path JAVA_JVM_TERMINATION_METHODS = FileTools.resolveOnResources("templates", "architecture" , "java", "archunit", "methods", "jvm-termination-methods.txt");
     public static final Path JAVA_REFLECTION_METHODS = FileTools.resolveOnResources("templates", "architecture" , "java", "archunit", "methods", "reflection-methods.txt");
     public static final Path JAVA_COMMAND_EXECUTION_METHODS = FileTools.resolveOnResources("templates", "architecture" , "java", "archunit", "methods", "command-execution-methods.txt");
+    public static final Path JAVA_THREAD_CREATION_METHODS = FileTools.resolveOnResources("templates", "architecture" , "java", "archunit", "methods", "thread-creation-methods.txt");
 
     private FileHandlerConstants() {
         throw new UnsupportedOperationException("Ares Security Error (Reason: Ares-Code; Stage: Execution): FileHandlerConstants is a utility class and should not be instantiated.");

src/main/java/de/tum/cit/ase/ares/api/architecture/java/archunit/JavaArchUnitSecurityTestCase.java

@@ -89,7 +89,7 @@ public void executeArchitectureTestCase(JavaClasses classes) {
                                 .NO_CLASSES_SHOULD_ACCESS_NETWORK
                                 .check(classes);
                 case THREAD_CREATION ->
-                        throw new UnsupportedOperationException("Ares Security Error (Reason: Ares-Code; Stage: Execution): Thread creation not implemented yet.");
+                        JavaArchitectureTestCaseCollection.NO_CLASSES_SHOULD_CREATE_THREADS.check(classes);
                 case COMMAND_EXECUTION ->
                         JavaArchitectureTestCaseCollection.NO_CLASSES_SHOULD_EXECUTE_COMMANDS.check(classes);
                 default -> throw new UnsupportedOperationException("Not implemented yet");

src/main/java/de/tum/cit/ase/ares/api/architecture/java/archunit/postcompile/CustomClassResolver.java

@@ -1,10 +1,8 @@
 package de.tum.cit.ase.ares.api.architecture.java.archunit.postcompile;
 
 //<editor-fold desc="Imports">
-import com.tngtech.archunit.ArchConfiguration;
 import com.tngtech.archunit.core.domain.JavaClass;
 import com.tngtech.archunit.core.importer.ClassFileImporter;
-import com.tngtech.archunit.core.importer.resolvers.ClassResolverFromClasspath;
 
 import java.net.URL;
 import java.util.Optional;
@@ -32,7 +30,9 @@ private CustomClassResolver() {
      * @return The resolved class if it exists.
      */
     public static Optional<JavaClass> tryResolve(String typeName) {
-        ArchConfiguration.get().setClassResolver(ClassResolverFromClasspath.class);
+        if (typeName.startsWith("de.tum.cit.ase.ares.api.aop.java.aspectj.adviceandpointcut.JavaAspectJFileSystemAdviceDefinitions")) {
+            return Optional.empty();
+        }
         URL url = CustomClassResolver.class.getResource("/" + typeName.replace(".", "/") + ".class");
         try {
             if (url == null) {

src/main/java/de/tum/cit/ase/ares/api/architecture/java/archunit/postcompile/JavaArchitectureTestCaseCollection.java

@@ -146,4 +146,11 @@ public boolean test(JavaAccess<?> javaAccess) {
             FileHandlerConstants.JAVA_COMMAND_EXECUTION_METHODS
     );
     //</editor-fold>
+
+    //<editor-fold desc="Thread Creation related rule">
+    public static final ArchRule NO_CLASSES_SHOULD_CREATE_THREADS = createNoClassShouldHaveMethodRule(
+            "creates threads",
+            FileHandlerConstants.JAVA_THREAD_CREATION_METHODS
+    );
+    //</editor-fold>
 }

src/main/java/de/tum/cit/ase/ares/api/architecture/java/archunit/postcompile/TransitivelyAccessesMethodsCondition.java

@@ -130,6 +130,7 @@ private boolean addAccessesToPathFrom(
         }
 
         /**
+         * We are currently using CHA, which resolves all the subclasses of a given class
          * @return all accesses to the same target as the supplied item that are not in the analyzed classes
          */
         private Set<JavaAccess<?>> getDirectAccessTargetsOutsideOfAnalyzedClasses(JavaAccess<?> item) {
@@ -163,8 +164,7 @@ private Set<JavaAccess<?>> getAccessesFromClass(JavaClass javaClass, String meth
                             .getOrigin()
                             .getFullName()
                             .substring(javaClass.getFullName().length())
-                            .equals(methodName)
-                            && isExceptionOrError(a.getTargetOwner()))
+                            .equals(methodName))
                     .collect(toSet());
         }
 

src/main/java/de/tum/cit/ase/ares/api/jupiter/JupiterSecurityExtension.java

@@ -24,6 +24,7 @@ public final class JupiterSecurityExtension implements UnifiedInvocationIntercep
     public <T> T interceptGenericInvocation(Invocation<T> invocation, ExtensionContext extensionContext,
                                             Optional<ReflectiveInvocationContext<?>> invocationContext) throws Throwable {
         JupiterContext testContext = JupiterContext.of(extensionContext);
+
         /**
          * Check if the test method has the {@link Policy} annotation. If it does, read
          * the policy file and run the security test cases.

src/main/java/de/tum/cit/ase/ares/api/securitytest/java/JavaSecurityTestCaseFactoryAndBuilder.java

@@ -324,11 +324,6 @@ public void executeSecurityTestCases() {
         @Nonnull JavaClasses classes = new ClassFileImporter().importPath(Paths.get(ProjectSourcesFinder.isGradleProject() ? "build" : "target", projectPath.toString()).toString());
         //</editor-fold>#
 
-        //<editor-fold desc="Set ArchUnit properties for optimization">
-        ArchConfiguration.get().setProperty("archRule.failOnEmptyShould", "false");
-        ArchConfiguration.get().setResolveMissingDependenciesFromClassPath(false);
-        //</editor-fold>
-
         //<editor-fold desc="Enforce fixed rules code">
         JavaArchitectureTestCaseCollection.NO_CLASSES_SHOULD_USE_REFLECTION.check(classes);
         JavaArchitectureTestCaseCollection.NO_CLASSES_SHOULD_TERMINATE_JVM.check(classes);

src/main/resources/de/tum/cit/ase/ares/api/templates/architecture/java/archunit/methods/file-system-access-methods.txt

@@ -34,7 +34,6 @@ java.io.PrintWriter.<init>(java.lang.String)
 java.io.PrintWriter.<init>(java.lang.String, java.lang.String)
 java.io.PrintWriter.<init>(java.lang.String, java.nio.charset.Charset)
 java.io.RandomAccessFile
-java.lang.ProcessImpl
 java.lang.Runtime.exec([Ljava.lang.String;, [Ljava.lang.String;, java.io.File)
 java.lang.Runtime.exec(java.lang.String, [Ljava.lang.String;, java.io.File)
 java.net.http.HttpRequest$BodyPublishers.ofFile(java.nio.file.Path)
@@ -44,7 +43,7 @@ java.net.http.HttpResponse$BodyHandlers.ofFileDownload(java.nio.file.Path, [Ljav
 java.net.http.HttpResponse$BodySubscribers.ofFile(java.nio.file.Path)
 java.net.http.HttpResponse$BodySubscribers.ofFile(java.nio.file.Path, [Ljava.nio.file.OpenOption;)
 java.nio.channels.FileChannel
-java.nio.file
+java.nio.file.Files
 java.security.KeyStore$Builder.newInstance(java.io.File, java.security.KeyStore$ProtectionParameter)
 java.security.KeyStore$Builder.newInstance(java.lang.String, java.security.Provider, java.io.File, java.security.KeyStore$ProtectionParameter)
 java.security.KeyStore.getInstance(java.io.File, [C)
@@ -67,15 +66,7 @@ java.util.logging.FileHandler.<init>(java.lang.String, int, int, boolean)
 java.util.logging.FileHandler.<init>(java.lang.String, long, int, boolean)
 java.util.logging.FileHandler.close()
 java.util.prefs
-java.util.prefs
 java.util.zip
-java.util.zip.ZipFile.<init>(java.io.File)
-java.util.zip.ZipFile.<init>(java.io.File, int)
-java.util.zip.ZipFile.<init>(java.io.File, int, java.nio.charset.Charset)
-java.util.zip.ZipFile.<init>(java.io.File, int, java.nio.charset.Charset)
-java.util.zip.ZipFile.<init>(java.io.File, java.nio.charset.Charset)
-java.util.zip.ZipFile.<init>(java.lang.String)
-java.util.zip.ZipFile.<init>(java.lang.String, java.nio.charset.Charset)
 javax.imageio
 javax.imageio.ImageIO.createImageInputStream(java.lang.Object)
 javax.imageio.ImageIO.createImageOutputStream(java.lang.Object)
@@ -92,18 +83,6 @@ javax.imageio.stream.FileCacheImageOutputStream.<init>(java.io.OutputStream, jav
 javax.imageio.stream.FileCacheImageOutputStream.close()
 javax.imageio.stream.FileImageInputStream.<init>(java.io.File)
 javax.imageio.stream.FileImageOutputStream.<init>(java.io.File)
-javax.management.loading.MLet.<init>()
-javax.management.loading.MLet.<init>([Ljava.net.URL;)
-javax.management.loading.MLet.<init>([Ljava.net.URL;, boolean)
-javax.management.loading.MLet.<init>([Ljava.net.URL;, java.lang.ClassLoader)
-javax.management.loading.MLet.<init>([Ljava.net.URL;, java.lang.ClassLoader, boolean)
-javax.management.loading.MLet.<init>([Ljava.net.URL;, java.lang.ClassLoader, java.net.URLStreamHandlerFactory)
-javax.management.loading.MLet.<init>([Ljava.net.URL;, java.lang.ClassLoader, java.net.URLStreamHandlerFactory, boolean)
-javax.management.loading.MLet.getMBeansFromURL(java.lang.String)
-javax.management.loading.MLet.getMBeansFromURL(java.net.URL)
-javax.management.loading.PrivateMLet.<init>([Ljava.net.URL;, boolean)
-javax.management.loading.PrivateMLet.<init>([Ljava.net.URL;, java.lang.ClassLoader, boolean)
-javax.management.loading.PrivateMLet.<init>([Ljava.net.URL;, java.lang.ClassLoader, java.net.URLStreamHandlerFactory, boolean)
 javax.management.modelmbean.RequiredModelMBean.sendAttributeChangeNotification(javax.management.Attribute, javax.management.Attribute)
 javax.management.modelmbean.RequiredModelMBean.sendAttributeChangeNotification(javax.management.AttributeChangeNotification)
 javax.management.modelmbean.RequiredModelMBean.sendNotification(java.lang.String)

src/main/resources/de/tum/cit/ase/ares/api/templates/architecture/java/archunit/methods/thread-creation-methods.txt

@@ -0,0 +1,3 @@
+java.lang.Thread.start
+java.lang.Thread.run
+java.lang.Thread.unstarted