diff --git a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java b/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java index 55188cb3..b4b05c16 100644 --- a/org-code-javabuilder/lib/src/main/java/org/code/javabuilder/JavabuilderSecurityPolicy.java +++ b/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), *
  • never allows execute *
  • denies all other filesystem access + *
  • denies replacing or removing the SecurityManager and this policy. * * *

    NOTE: {@link SecurityManager} / {@link Policy} are deprecated in Java 17 and removed in Java @@ -62,6 +64,14 @@ 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; } @@ -69,12 +79,11 @@ public boolean implies(ProtectionDomain domain, Permission permission) { * 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. + *

    #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) { diff --git a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java b/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java index 825d8dd0..4e4f1fa2 100644 --- a/org-code-javabuilder/lib/src/test/java/org/code/javabuilder/JavabuilderSecurityPolicyTest.java +++ b/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"))); }