From c13aab927fa11aa451a188bf3f6b183589394bf7 Mon Sep 17 00:00:00 2001
From: Duarte Meneses <duarte.meneses@sonarsource.com>
Date: Thu, 31 Oct 2019 10:56:28 -0500
Subject: [PATCH] SONAR-12617 Security restrictions prevent plugins from
 reading environment variables

---
 .../org/sonar/process/SecurityManagement.java | 20 ++++++++++++++-----
 .../sonar/process/SecurityManagementTest.java | 13 ++++++------
 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();
   }
 }
-- 
2.39.5