packages/derby
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
derby/CVE-2018-1313.patch
wangxiao65 8f08e6696f fix CVE-2018-1313
2020-11-11 14:42:18 +08:00

541 lines
24 KiB
Diff
Raw Blame History

From a2027c64e185a9ce46929f352e2db03371c1f95b Mon Sep 17 00:00:00 2001
From: Bryan Pendleton <bpendleton@apache.org>
Date: Sun, 11 Mar 2018 21:50:26 +0000
Subject: [PATCH] Remove support for COMMAND_TESTCONNECTION connectToDatabase
feature. It was never documented, and was never part of the regression tests.
A new regression test demonstrates that attempting to use this feature
now results in a "usage" response from the Network Server.
Also, change the Network Server's default security policy file, as well
as the template security policy file that we distribute with the
release and in the documentation. The new policy files do not use
the <<ALL FILES>> permission; instead they restrict the permission
to certain directories only.
Also, change the implementation of the Network Server's "sysinfo"
sub-command. Prior to this change, that feature would open each
jar file on the Network Server's class path, and would report on
whatever information it could find about that jar file. Now, the
only jar files that are opened and reported on are the known Derby
jar files; a table of those jar file names is compiled into the
sysinfo command. This causes sysinfo to return less information,
but it also means that it only needs the permission to access the
known Derby jar files on the classpath.
git-svn-id: https://svn.apache.org/repos/asf/db/derby/code/branches/10.14@1826467 13f79535-47bb-0310-9956-ffa450edef68
---
.../derby/drda/NetworkServerControl.java | 24 +++-
java/drda/org/apache/derby/drda/server.policy | 24 +---
.../org/apache/derby/drda/template.policy | 18 ++-
.../impl/drda/NetworkServerControlImpl.java | 48 +------
.../apache/derby/iapi/reference/Property.java | 1 +
.../derbynet/NetworkServerControlApiTest.java | 136 ++++++++++++++++++
.../NetworkServerControlApiTest.policy | 4 +
.../apache/derby/impl/tools/sysinfo/Main.java | 41 ++++++
8 files changed, 222 insertions(+), 74 deletions(-)
diff --git a/java/drda/org/apache/derby/drda/NetworkServerControl.java b/java/drda/org/apache/derby/drda/NetworkServerControl.java
index 73a925b1ef..103342ec87 100644
--- a/java/drda/org/apache/derby/drda/NetworkServerControl.java
+++ b/java/drda/org/apache/derby/drda/NetworkServerControl.java
@@ -21,6 +21,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
package org.apache.derby.drda;
+import java.io.File;
import java.io.PrintWriter;
import java.net.Inet6Address;
import java.net.InetAddress;
@@ -686,10 +687,14 @@ private static void installSecurityManager( NetworkServerControlImpl server )
// network codesources. Do not let the customer
// override this
//
- String derbyInstallURL = getCodeSourcePrefix( server );
+ URL derbyInstallURL = getCodeSourceURL( server );
+ String derbyInstallStr = getCodeSourcePrefix( server, derbyInstallURL );
+ String derbyInstallPth = new File(derbyInstallURL.getFile())
+ .getParentFile().getAbsolutePath();
+
+ System.setProperty( Property.DERBY_INSTALL_URL, derbyInstallStr );
+ System.setProperty( Property.DERBY_INSTALL_PATH, derbyInstallPth );
- System.setProperty( Property.DERBY_INSTALL_URL, derbyInstallURL );
-
//
// Now install a SecurityManager, using the Basic policy file.
//
@@ -790,7 +795,7 @@ private static boolean isIPV6Address( String hostname )
* same directory.
* </p>
*/
- private static String getCodeSourcePrefix( NetworkServerControlImpl server )
+ private static URL getCodeSourceURL( NetworkServerControlImpl server )
throws Exception
{
// Note: This method is expected to run only when no security manager
@@ -801,9 +806,14 @@ private static String getCodeSourcePrefix( NetworkServerControlImpl server )
return null;
}
URL url = cs.getLocation();
- if (url == null) {
- return null;
- }
+ return url;
+ }
+
+ private static String getCodeSourcePrefix(
+ NetworkServerControlImpl server,
+ URL url )
+ throws Exception
+ {
// Replace in "file://some", but not in "file:///some".
String extForm = url.toExternalForm().replaceFirst(
"^file://([^/].*)", "file:////$1");
diff --git a/java/drda/org/apache/derby/drda/server.policy b/java/drda/org/apache/derby/drda/server.policy
index 30c321cb59..a91c079624 100644
--- a/java/drda/org/apache/derby/drda/server.policy
+++ b/java/drda/org/apache/derby/drda/server.policy
@@ -32,17 +32,7 @@ grant codeBase "${derby.install.url}derby.jar"
permission java.io.FilePermission "${derby.system.home}${/}-",
"read,write,delete";
- // This permission lets you backup and restore databases to and from
- // arbitrary locations in your file system.
- //
- // This permission also lets you import/export data to and from arbitrary
- // locations in your file system.
- //
- // You may want to restrict this access to specific directories.
- //
- permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete";
-
- // Needed by sysinfo. The file permission is needed to check the existence of
+ // Needed by sysinfo. A file permission is needed to check the existence of
// jars on the classpath. You can limit this permission to just the locations
// which hold your jar files. This block is reproduced for all codebases
// which include the sysinfo classes--the policy file syntax does not let you
@@ -54,9 +44,9 @@ grant codeBase "${derby.install.url}derby.jar"
permission java.util.PropertyPermission "java.runtime.version", "read";
permission java.util.PropertyPermission "java.fullversion", "read";
permission java.lang.RuntimePermission "getProtectionDomain";
- permission java.io.FilePermission "<<ALL FILES>>", "read";
permission java.io.FilePermission "java.runtime.version", "read";
permission java.io.FilePermission "java.fullversion", "read";
+ permission java.io.FilePermission "${derby.install.path}${/}-", "read";
// Permissions needed for JMX based management and monitoring.
//
@@ -143,7 +133,7 @@ grant codeBase "${derby.install.url}derbynet.jar"
"control,monitor";
permission org.apache.derby.security.SystemPermission "engine", "usederbyinternals";
- // Needed by sysinfo. The file permission is needed to check the existence of
+ // Needed by sysinfo. A file permission is needed to check the existence of
// jars on the classpath. You can limit this permission to just the locations
// which hold your jar files. This block is reproduced for all codebases
// which include the sysinfo classes--the policy file syntax does not let you
@@ -155,15 +145,15 @@ grant codeBase "${derby.install.url}derbynet.jar"
permission java.util.PropertyPermission "java.runtime.version", "read";
permission java.util.PropertyPermission "java.fullversion", "read";
permission java.lang.RuntimePermission "getProtectionDomain";
- permission java.io.FilePermission "<<ALL FILES>>", "read";
permission java.io.FilePermission "java.runtime.version", "read";
permission java.io.FilePermission "java.fullversion", "read";
+ permission java.io.FilePermission "${derby.install.path}${/}-", "read";
};
grant codeBase "${derby.install.url}derbytools.jar"
{
- // Needed by sysinfo. The file permission is needed to check the existence of
+ // Needed by sysinfo. A file permission is needed to check the existence of
// jars on the classpath. You can limit this permission to just the locations
// which hold your jar files. This block is for all codebases which include
// the sysinfo classes--the policy file syntax does not let you grant
@@ -182,7 +172,7 @@ grant codeBase "${derby.install.url}derbytools.jar"
grant codeBase "${derby.install.url}derbyclient.jar"
{
- // Needed by sysinfo. The file permission is needed to check the existence of
+ // Needed by sysinfo. A file permission is needed to check the existence of
// jars on the classpath. You can limit this permission to just the locations
// which hold your jar files. This block is reproduced for all codebases
// which include the sysinfo classes--the policy file syntax does not let you
@@ -194,7 +184,7 @@ grant codeBase "${derby.install.url}derbyclient.jar"
permission java.util.PropertyPermission "java.runtime.version", "read";
permission java.util.PropertyPermission "java.fullversion", "read";
permission java.lang.RuntimePermission "getProtectionDomain";
- permission java.io.FilePermission "<<ALL FILES>>", "read";
+ permission java.io.FilePermission "${derby.install.path}${/}-", "read";
// The following permission must be granted for Connection.abort(Executor) to
// work. Note that this permission must also be granted to outer
diff --git a/java/drda/org/apache/derby/drda/template.policy b/java/drda/org/apache/derby/drda/template.policy
index 6331564216..60d811e730 100644
--- a/java/drda/org/apache/derby/drda/template.policy
+++ b/java/drda/org/apache/derby/drda/template.policy
@@ -52,9 +52,13 @@ grant codeBase "${derby.install.url}derby.jar"
// This permission also lets you import/export data to and from arbitrary
// locations in your file system.
//
- // You may want to restrict this access to specific directories.
+ // NOTE: this permission is commented out. You should NOT grant blanket
+ // permission to the entire filesystem! If you choose to use this
+ // permission to allow the server to access files outside of the
+ // server's home directory, you should name those specific directories
+ // in the permisson (that is, do NOT specify ALL FILES).
//
- permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete";
+ // permission java.io.FilePermission "<<ALL FILES>>", "read,write,delete";
// Permissions needed for JMX based management and monitoring.
//
@@ -139,9 +143,11 @@ grant codeBase "${derby.install.url}derbynet.jar"
//
// permission java.net.SocketPermission "*", "connect,resolve";
- // Needed by sysinfo. The file permission is needed to check the existence of
- // jars on the classpath. You can limit this permission to just the locations
- // which hold your jar files.
+ // Needed by sysinfo. A file permission is needed to check the existence of
+ // jars on the classpath. Note that this permission is commented out!
+ // You should limit this permission to just the locations which hold
+ // your jar files; do NOT grant blanket permission to read the entire
+ // filesystem.
//
// In this template file, this block of permissions is granted to
// derbynet.jar under the assumption that derbynet.jar is the first jar file
@@ -155,12 +161,12 @@ grant codeBase "${derby.install.url}derbynet.jar"
// derbyclient.jar
// derbytools.jar
//
+ // permission java.io.FilePermission "${derby.install.directory}${/}-", "read";
permission java.util.PropertyPermission "user.*", "read";
permission java.util.PropertyPermission "java.home", "read";
permission java.util.PropertyPermission "java.class.path", "read";
permission java.util.PropertyPermission "java.runtime.version", "read";
permission java.util.PropertyPermission "java.fullversion", "read";
permission java.lang.RuntimePermission "getProtectionDomain";
- permission java.io.FilePermission "<<ALL FILES>>", "read";
};
diff --git a/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java b/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
index fc624276df..2f5e9ccde8 100644
--- a/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
+++ b/java/drda/org/apache/derby/impl/drda/NetworkServerControlImpl.java
@@ -1820,11 +1820,11 @@ protected synchronized void processCommands(DDMReader reader, DDMWriter writer,
consolePropertyMessage("DRDA_TraceDirectoryChange.I", traceDirectory);
break;
case COMMAND_TESTCONNECTION:
- databaseArg = reader.readCmdString();
- userArg = reader.readCmdString();
- passwordArg = reader.readCmdString();
+ databaseArg = reader.readCmdString(); // This is ...
+ userArg = reader.readCmdString(); // ... no longer ...
+ passwordArg = reader.readCmdString(); // ... supported.
if (databaseArg != null)
- connectToDatabase(writer, databaseArg, userArg, passwordArg);
+ sendMessage(writer, ERROR, "Usage: ping()");
else
sendOK(writer);
break;
@@ -3931,46 +3931,6 @@ private void setTraceDirectory(String value)
- /**
- * Connect to a database to test whether a connection can be made
- *
- * @param writer connection to send message to
- * @param database database directory to connect to
- * @param user user to use
- * @param password password to use
- */
- private void connectToDatabase(DDMWriter writer, String database, String user,
- String password) throws Exception
- {
- Properties p = new Properties();
- if (user != null)
- p.put("user", user);
- if (password != null)
- p.put("password", password);
- try {
- Class.forName(CLOUDSCAPE_DRIVER);
- }
- catch (Exception e) {
- sendMessage(writer, ERROR, e.getMessage());
- return;
- }
- try {
- //Note, we add database to the url so that we can allow additional
- //url attributes
- Connection conn = getDriver().connect(Attribute.PROTOCOL+database, p);
- // send warnings
- SQLWarning warn = conn.getWarnings();
- if (warn != null)
- sendSQLMessage(writer, warn, SQLWARNING);
- else
- sendOK(writer);
- conn.close();
- return;
- } catch (SQLException se) {
- sendSQLMessage(writer, se, SQLERROR);
- }
- }
-
/**
* Wrap SQL Error - display to console and raise exception
*
diff --git a/java/engine/org/apache/derby/iapi/reference/Property.java b/java/engine/org/apache/derby/iapi/reference/Property.java
index bda045629a..ecf9bd9a5b 100644
--- a/java/engine/org/apache/derby/iapi/reference/Property.java
+++ b/java/engine/org/apache/derby/iapi/reference/Property.java
@@ -416,6 +416,7 @@ Default value for wait timeouts (60 seconds)
* This property is the location of the derby jars.
**/
public static final String DERBY_INSTALL_URL = "derby.install.url";
+ public static final String DERBY_INSTALL_PATH = "derby.install.path";
/**
* This property is private to Derby.
diff --git a/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.java b/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.java
index a10c41b595..e50cd5e581 100644
--- a/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.java
+++ b/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.java
@@ -22,6 +22,10 @@ Licensed to the Apache Software Foundation (ASF) under one or more
package org.apache.derbyTesting.functionTests.tests.derbynet;
import java.io.File;
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import javax.net.SocketFactory;
+import java.net.Socket;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.security.AccessController;
@@ -205,6 +209,138 @@ public void test_03_Ping() throws Exception
// expected exception
}
}
+
+ /*
+ * CVE-2018-1313: Attempt to pass arguments to COMMAND_TESTCONNECTION
+ */
+ public void test_03_ping_args() throws Exception
+ {
+ String response = tryPingDbError("mydatabase", "myuser", "mypassword");
+ //System.out.println(response);
+ // This once said: XJ004:Database 'mydatabase' not found.
+ assertEquals("Usage", response.substring(0,5));
+
+ response = tryPingDbError("some/sorta/db","someone","somecredentials");
+ //System.out.println(response);
+ assertEquals("Usage", response.substring(0,5));
+
+ response = tryPingDbError("\\\\192.168.1.2\\guest\\db1","tata","tata");
+ //System.out.println(response);
+ assertEquals("Usage", response.substring(0,5));
+
+ response = tryPingDbError("my/nocred/db", "", "");
+ //System.out.println(response);
+ assertEquals("Usage", response.substring(0,5));
+
+ response = tryPingDbOK("", "scarface", "evildoer");
+ //System.out.println(response);
+ assertEquals("OK", response.substring(0,2));
+ }
+
+ private Socket privilegedClientSocket(final String host, int port)
+ throws Exception
+ {
+ try {
+ return AccessController.doPrivileged(
+ new PrivilegedExceptionAction<Socket>() {
+ public Socket run() throws Exception {
+ return SocketFactory.getDefault().createSocket(
+ InetAddress.getByName(host), port);
+ }
+ });
+ } catch (PrivilegedActionException pae) {
+ throw (Exception)pae.getCause();
+ }
+ }
+
+ private static String byteArrayToHex(byte[] ba, int l)
+ {
+ if (l < 0) return "STRING OF NEGATIVE LENGTH("+l+")";
+ StringBuilder sb = new StringBuilder(l * 2);
+ for (int i = 0; i < l; i++) sb.append(String.format("%02x", ba[i]));
+ return sb.toString();
+ }
+
+ private String tryPingDbError(String d, String u, String p)
+ throws Exception
+ {
+ return tryPingDbTest(2, d, u, p); // Result 2: ERROR
+ }
+
+ private String tryPingDbOK(String d, String u, String p)
+ throws Exception
+ {
+ return tryPingDbTest(0, d, u, p); // Result 0: OK
+ }
+
+ private String tryPingDbTest(int rc, String d, String u, String p)
+ throws Exception
+ {
+ //System.out.println("database: '"+d+"' (len: "+d.length()+")");
+ //System.out.println(" user: '"+u+"' (len: "+u.length()+")");
+ //System.out.println("password: '"+p+"' (len: "+p.length()+")");
+
+ Socket clientSocket = privilegedClientSocket(
+ TestConfiguration.getCurrent().getHostName(),
+ TestConfiguration.getCurrent().getPort());
+ ByteArrayOutputStream byteArrayOs = new ByteArrayOutputStream();
+ DataOutputStream commandOs = new DataOutputStream(byteArrayOs);
+
+ byte[] msgBytes = "CMD:".getBytes("UTF8");
+ commandOs.write(msgBytes,0,msgBytes.length);
+ commandOs.writeByte((byte) 0); // default version: 02
+ commandOs.writeByte((byte) 2); // default version: 02
+ commandOs.writeByte((byte) 0); // default locale: 0
+ commandOs.writeByte((byte) 0); // default codeset: 0
+ commandOs.writeByte((byte) 4); // COMMAND_TESTCONNECTION
+
+ msgBytes = d.getBytes("UTF8");
+ commandOs.writeByte((byte)(msgBytes.length >> 8 ));
+ commandOs.writeByte((byte) msgBytes.length);
+ commandOs.write(msgBytes,0,msgBytes.length);
+
+ msgBytes = u.getBytes("UTF8");
+ commandOs.writeByte((byte)(msgBytes.length >> 8 ));
+ commandOs.writeByte((byte) msgBytes.length);
+ commandOs.write(msgBytes,0,msgBytes.length);
+
+ msgBytes = p.getBytes("UTF8");
+ commandOs.writeByte((byte)(msgBytes.length >> 8 ));
+ commandOs.writeByte((byte) msgBytes.length);
+ commandOs.write(msgBytes,0,msgBytes.length);
+
+ byteArrayOs.writeTo(clientSocket.getOutputStream());
+ commandOs.flush();
+ byteArrayOs.reset();
+ clientSocket.shutdownOutput();
+
+ byte[]result = new byte[1024];
+ int resultLen = clientSocket.getInputStream().read(result);
+
+ clientSocket.close();
+
+ //System.out.println( "Result was " + resultLen + " bytes long");
+ //System.out.println( byteArrayToHex(result,resultLen) );
+
+ if (resultLen < 0)
+ return "DISCONNECT";
+
+ String r = "RPY:";
+ int rl = r.length();
+ assertTrue(resultLen > rl);
+ String header = new String(result, 0, rl, "UTF8");
+ assertEquals(r, header);
+ assertEquals(rc, result[rl++]); // 0: OK, 2: ERROR, 3: SQLERROR, etc.
+
+ if (rc == 0)
+ return "OK";
+
+ int l = ((result[rl++] & 0xff) << 8) + (result[rl++] & 0xff);
+ String response = new String(result, rl, l, "UTF8");
+
+ return response;
+ }
+
/**
* Wraps InitAddress.getByName in privilege block.
diff --git a/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.policy b/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.policy
index 792bcea43f..50f894a718 100644
--- a/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.policy
+++ b/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/NetworkServerControlApiTest.policy
@@ -186,6 +186,10 @@ grant codeBase "${derbyTesting.testjar}derbyTesting.jar" {
// Needed by NetworkServerTestSetup when probing ports.
permission java.net.SocketPermission "localhost", "listen";
+ permission java.net.SocketPermission "127.0.0.1", "accept,connect,resolve";
+ permission java.net.SocketPermission "localhost", "accept,connect,listen";
+ permission java.net.SocketPermission "${derbyTesting.clienthost}", "accept,connect";
+ permission java.net.SocketPermission "${derbyTesting.serverhost}", "accept,connect";
};
//
diff --git a/java/tools/org/apache/derby/impl/tools/sysinfo/Main.java b/java/tools/org/apache/derby/impl/tools/sysinfo/Main.java
index e332ded977..6e88063202 100644
--- a/java/tools/org/apache/derby/impl/tools/sysinfo/Main.java
+++ b/java/tools/org/apache/derby/impl/tools/sysinfo/Main.java
@@ -23,6 +23,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.Arrays;
+import java.util.List;
import java.util.Locale;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
@@ -889,6 +891,31 @@ private static String argumentMatches(String[] args, String ss) {
".properties",
};
+ private static final String jarNames[] =
+ {
+ "derby.jar",
+ "derbyclient.jar",
+ "derbynet.jar",
+ "derbyoptionaltools.jar",
+ "derbyrun.jar",
+ "derbyshared.jar",
+ "derbyTesting.jar",
+ "derbytools.jar",
+ "derbyLocale_cs.jar",
+ "derbyLocale_de_DE.jar",
+ "derbyLocale_es.jar",
+ "derbyLocale_ja_JP.jar",
+ "derbyLocale_ko_KR.jar",
+ "derbyLocale_pl.jar",
+ "derbyLocale_pt_BR.jar",
+ "derbyLocale_ru.jar",
+ "derbyLocale_fr.jar",
+ "derbyLocale_zh_CN.jar",
+ "derbyLocale_hu.jar",
+ "derbyLocale_zh_TW.jar",
+ "derbyLocale_it.jar"
+ };
+
/**
* Get all the info we can obtain from the local execution context
* as to the availability of the Derby classes by attempting to load
@@ -920,9 +947,23 @@ private static String argumentMatches(String[] args, String ss) {
{
if (classpath != null) {
String cp [] = parseClasspath(classpath);
+ List<String> jarNamesList = Arrays.asList(jarNames);
Vector<ZipInfoProperties> v = new Vector<ZipInfoProperties>();
for (int i = 0; i < cp.length; i++)
{
+ boolean matches = false;
+ String candidate = cp[i];
+ for (String jarName : jarNames)
+ {
+ if (candidate.endsWith(jarName))
+ {
+ matches = true;
+ break;
+ }
+ }
+ if (!matches)
+ continue;
+
ZipInfoProperties zip = null;
try {
zip = checkForInfo(cp[i]);
Reference in New Issue View Git Blame Copy Permalink