SONAR-12617 Security restrictions prevent plugins from reading environment variables

2 files changed, 21 insertions, 12 deletions

diff --git a/server/sonar-process/src/main/java/org/sonar/process/SecurityManagement.java b/server/sonar-process/src/main/java/org/sonar/process/SecurityManagement.java
index e8f49e0d6d8..7f1d6cad31c 100644
--- a/server/sonar-process/src/main/java/org/sonar/process/SecurityManagement.java
+++ b/server/sonar-process/src/main/java/org/sonar/process/SecurityManagement.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.process;
 
-import java.lang.reflect.ReflectPermission;
 import java.security.Permission;
 import java.security.Policy;
 import java.security.ProtectionDomain;
@@ -40,18 +39,29 @@ public class SecurityManagement {
   }
 
   static class CustomPolicy extends Policy {
-    private static final Set<String> ALLOWED_RUNTIME_PERMISSIONS = new HashSet<>(Arrays.asList("getFileSystemAttributes", "readFileDescriptor", "writeFileDescriptor",
-      "getStackTrace", "setDefaultUncaughtExceptionHandler", "manageProcess", "localeServiceProvider", "LoggerFinder"));
+    private static final Set<String> BLOCKED_RUNTIME_PERMISSIONS = new HashSet<>(Arrays.asList(
+      "createClassLoader",
+      "getClassLoader",
+      "setContextClassLoader",
+      "enableContextClassLoaderOverride",
+      "closeClassLoader",
+      "setSecurityManager",
+      "createSecurityManager"
+    ));
+    private static final Set<String> BLOCKED_SECURITY_PERMISSIONS = new HashSet<>(Arrays.asList(
+      "createAccessControlContext",
+      "setPolicy"
+    ));
 
     @Override
     public boolean implies(ProtectionDomain domain, Permission permission) {
       // classloader used to load plugins
       String clName = getDomainClassLoaderName(domain);
       if ("org.sonar.classloader.ClassRealm".equals(clName)) {
-        if (permission instanceof RuntimePermission && !ALLOWED_RUNTIME_PERMISSIONS.contains(permission.getName())) {
+        if (permission instanceof RuntimePermission && BLOCKED_RUNTIME_PERMISSIONS.contains(permission.getName())) {
           return false;
         }
-        if (permission instanceof ReflectPermission || permission instanceof SecurityPermission) {
+        if (permission instanceof SecurityPermission && BLOCKED_SECURITY_PERMISSIONS.contains(permission.getName())) {
           return false;
         }
       }
diff --git a/server/sonar-process/src/test/java/org/sonar/process/SecurityManagementTest.java b/server/sonar-process/src/test/java/org/sonar/process/SecurityManagementTest.java
index afe142b4ea6..92060015f53 100644
--- a/server/sonar-process/src/test/java/org/sonar/process/SecurityManagementTest.java
+++ b/server/sonar-process/src/test/java/org/sonar/process/SecurityManagementTest.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.process;
 
-import java.lang.reflect.ReflectPermission;
 import java.security.Permission;
 import java.security.ProtectionDomain;
 import java.security.SecurityPermission;
@@ -35,8 +34,8 @@ public class SecurityManagementTest {
 
   private Permission allowedRuntime = new RuntimePermission("getFileSystemAttributes");
   private Permission deniedRuntime = new RuntimePermission("getClassLoader");
-  private Permission reflect = new ReflectPermission("suppressAccessChecks");
-  private Permission security = new SecurityPermission("setPolicy");
+  private Permission allowedSecurity = new SecurityPermission("getProperty.key");
+  private Permission deniedSecurity = new SecurityPermission("setPolicy");
 
   @Test
   public void policy_restricts_class_realm() {
@@ -47,10 +46,10 @@ public class SecurityManagementTest {
       }
     };
 
+    assertThat(policy.implies(pd, allowedSecurity)).isTrue();
+    assertThat(policy.implies(pd, deniedSecurity)).isFalse();
     assertThat(policy.implies(pd, allowedRuntime)).isTrue();
     assertThat(policy.implies(pd, deniedRuntime)).isFalse();
-    assertThat(policy.implies(pd, reflect)).isFalse();
-    assertThat(policy.implies(pd, security)).isFalse();
   }
 
   @Test
@@ -62,9 +61,9 @@ public class SecurityManagementTest {
       }
     };
 
+    assertThat(policy.implies(pd, allowedSecurity)).isTrue();
+    assertThat(policy.implies(pd, deniedSecurity)).isTrue();
     assertThat(policy.implies(pd, allowedRuntime)).isTrue();
     assertThat(policy.implies(pd, deniedRuntime)).isTrue();
-    assertThat(policy.implies(pd, reflect)).isTrue();
-    assertThat(policy.implies(pd, security)).isTrue();
   }
 }