Fix for BC-121

org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java

@@ -7,6 +7,7 @@
 import java.security.Permission;
 import java.security.Policy;
 import java.security.ProtectionDomain;
+import java.security.SecurityPermission;
 import java.security.cert.Certificate;
 import java.util.List;
 
@@ -24,6 +25,7 @@
  *       (so class loading, fonts, and bundled assets keep working),
  *   <li>never allows execute
  *   <li>denies all other filesystem access
+ *   <li>denies replacing or removing the SecurityManager and this policy.
  * </ul>
  *
  * <p>NOTE: {@link SecurityManager} / {@link Policy} are deprecated in Java 17 and removed in Java
@@ -62,19 +64,26 @@ public boolean implies(ProtectionDomain domain, Permission permission) {
     if (permission instanceof FilePermission) {
       return isAllowedFileAccess((FilePermission) permission);
     }
+
+    if (permission instanceof RuntimePermission
+        && "setSecurityManager".equals(permission.getName())) {
+      return false;
+    }
+    if (permission instanceof SecurityPermission && "setPolicy".equals(permission.getName())) {
+      return false;
+    }
     return true;
   }
 
   /**
    * Forces every class that #implies references to load. Called from the constructor so it always
    * runs before a SecurityManager is installed.
    *
-   * #implies runs on every permission check, including the File.exists() calls
-   * the class loader makes to locate a class. If a class #implies references
-   * is still unloaded when the first such check fires, loading it re-enters
-   * #implies before the load completes and throws ClassCircularityError. Exercising
-   * the policy here, while no SecurityManager is active, guarantees those classes are already
-   * loaded by the time checks begin.
+   * <p>#implies runs on every permission check, including the File.exists() calls the class loader
+   * makes to locate a class. If a class #implies references is still unloaded when the first such
+   * check fires, loading it re-enters #implies before the load completes and throws
+   * ClassCircularityError. Exercising the policy here, while no SecurityManager is active,
+   * guarantees those classes are already loaded by the time checks begin.
    */
   private void warmUp() {
     final ProtectionDomain studentDomain =
@@ -89,6 +98,8 @@ private void warmUp() {
             null);
     this.implies(studentDomain, new FilePermission("warmup", "read"));
     this.implies(studentDomain, new FilePermission("warmup", "write"));
+    this.implies(studentDomain, new RuntimePermission("setSecurityManager"));
+    this.implies(studentDomain, new SecurityPermission("setPolicy"));
   }
 
   private boolean isConfinedStudentCode(ProtectionDomain domain) {

org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java

@@ -6,6 +6,7 @@
 import java.net.URL;
 import java.security.CodeSource;
 import java.security.ProtectionDomain;
+import java.security.SecurityPermission;
 import java.security.cert.Certificate;
 import java.util.List;
 import java.util.PropertyPermission;
@@ -76,6 +77,19 @@ public void studentCannotExecuteEvenUnderTmp() {
     assertFalse(policy.implies(validatorDomain, new FilePermission(tmpBinary, "execute")));
   }
 
+  @Test
+  public void studentCannotDisableTheSandbox() {
+    // System.setSecurityManager(null) would remove the sandbox; Policy.setPolicy() would let
+    // student code replace this policy with an allow-everything one.
+    assertFalse(policy.implies(userDomain, new RuntimePermission("setSecurityManager")));
+    assertFalse(policy.implies(userDomain, new SecurityPermission("setPolicy")));
+    assertFalse(policy.implies(validatorDomain, new RuntimePermission("setSecurityManager")));
+    assertFalse(policy.implies(validatorDomain, new SecurityPermission("setPolicy")));
+    // Framework code (LambdaRequestHandler) installs the manager and policy itself.
+    assertTrue(policy.implies(frameworkDomain, new RuntimePermission("setSecurityManager")));
+    assertTrue(policy.implies(frameworkDomain, new SecurityPermission("setPolicy")));
+  }
+
   @Test
   public void validatorRunIsAlsoConfined() {
     // The validation run loads and executes student code in a VALIDATOR-level UserClassLoader, so
@@ -88,7 +102,8 @@ public void validatorRunIsAlsoConfined() {
   @Test
   public void constructorWarmUpDoesNotThrowAndPolicyStillEnforces() {
     // Construction warms the policy up; it must not throw and must not weaken enforcement.
-    final JavabuilderSecurityPolicy freshPolicy = assertDoesNotThrow(JavabuilderSecurityPolicy::new);
+    final JavabuilderSecurityPolicy freshPolicy =
+        assertDoesNotThrow(JavabuilderSecurityPolicy::new);
     assertFalse(freshPolicy.implies(userDomain, new FilePermission("/etc/passwd", "read")));
   }