242 lines
12 KiB
Diff
242 lines
12 KiB
Diff
From: Markus Koschany <apo@debian.org>
|
|
Date: Wed, 28 Dec 2022 13:36:54 +0100
|
|
Subject: CVE-2021-37533
|
|
|
|
Bug-Debian: https://bugs.debian.org/1025910
|
|
Origin: https://github.com/apache/commons-net/commit/b0bff89f70cfea70009e22f87639816cc3993974
|
|
---
|
|
.../java/org/apache/commons/net/ftp/FTPClient.java | 91 ++++++++++++++++++----
|
|
.../org/apache/commons/net/ftp/FTPClientTest.java | 41 +++++++---
|
|
2 files changed, 106 insertions(+), 26 deletions(-)
|
|
|
|
diff --git a/src/main/java/org/apache/commons/net/ftp/FTPClient.java b/src/main/java/org/apache/commons/net/ftp/FTPClient.java
|
|
index bec47d1..16d969b 100644
|
|
--- a/src/main/java/org/apache/commons/net/ftp/FTPClient.java
|
|
+++ b/src/main/java/org/apache/commons/net/ftp/FTPClient.java
|
|
@@ -307,6 +307,18 @@ implements Configurable
|
|
*/
|
|
public static final String FTP_SYSTEM_TYPE_DEFAULT = "org.apache.commons.net.ftp.systemType.default";
|
|
|
|
+ /**
|
|
+ * The system property that defines the default for {@link #isIpAddressFromPasvResponse()}. This property, if present, configures the default for the
|
|
+ * following: If the client receives the servers response for a PASV request, then that response will contain an IP address. If this property is true, then
|
|
+ * the client will use that IP address, as requested by the server. This is compatible to version {@code 3.8.0}, and before. If this property is false, or
|
|
+ * absent, then the client will ignore that IP address, and instead use the remote address of the control connection.
|
|
+ *
|
|
+ * @see #isIpAddressFromPasvResponse()
|
|
+ * @see #setIpAddressFromPasvResponse(boolean)
|
|
+ * @since 3.9.0
|
|
+ */
|
|
+ public static final String FTP_IP_ADDRESS_FROM_PASV_RESPONSE = "org.apache.commons.net.ftp.ipAddressFromPasvResponse";
|
|
+
|
|
/**
|
|
* The name of an optional systemType properties file ({@value}), which is loaded
|
|
* using {@link Class#getResourceAsStream(String)}.<br>
|
|
@@ -506,6 +518,8 @@ implements Configurable
|
|
__featuresMap = null;
|
|
}
|
|
|
|
+ private boolean ipAddressFromPasvResponse = Boolean.parseBoolean(System.getProperty(FTPClient.FTP_IP_ADDRESS_FROM_PASV_RESPONSE));
|
|
+
|
|
/**
|
|
* Parse the pathname from a CWD reply.
|
|
* <p>
|
|
@@ -565,37 +579,43 @@ implements Configurable
|
|
{
|
|
java.util.regex.Matcher m = __PARMS_PAT.matcher(reply);
|
|
if (!m.find()) {
|
|
- throw new MalformedServerReplyException(
|
|
- "Could not parse passive host information.\nServer Reply: " + reply);
|
|
+ throw new MalformedServerReplyException("Could not parse passive host information.\nServer Reply: " + reply);
|
|
}
|
|
|
|
- __passiveHost = m.group(1).replace(',', '.'); // Fix up to look like IP address
|
|
+ int pasvPort;
|
|
+ // Fix up to look like IP address
|
|
+ String pasvHost = "0,0,0,0".equals(m.group(1)) ? _socket_.getInetAddress().getHostAddress() : m.group(1).replace(',', '.');
|
|
|
|
try
|
|
{
|
|
int oct1 = Integer.parseInt(m.group(2));
|
|
int oct2 = Integer.parseInt(m.group(3));
|
|
- __passivePort = (oct1 << 8) | oct2;
|
|
+ pasvPort = (oct1 << 8) | oct2;
|
|
}
|
|
catch (NumberFormatException e)
|
|
{
|
|
- throw new MalformedServerReplyException(
|
|
- "Could not parse passive port information.\nServer Reply: " + reply);
|
|
+ throw new MalformedServerReplyException("Could not parse passive port information.\nServer Reply: " + reply);
|
|
}
|
|
-
|
|
- if (__passiveNatWorkaroundStrategy != null) {
|
|
- try {
|
|
- String passiveHost = __passiveNatWorkaroundStrategy.resolve(__passiveHost);
|
|
- if (!__passiveHost.equals(passiveHost)) {
|
|
- fireReplyReceived(0,
|
|
- "[Replacing PASV mode reply address "+__passiveHost+" with "+passiveHost+"]\n");
|
|
- __passiveHost = passiveHost;
|
|
+ if (isIpAddressFromPasvResponse()) {
|
|
+ // Pre-3.9.0 behavior
|
|
+ if (__passiveNatWorkaroundStrategy != null) {
|
|
+ try {
|
|
+ final String newPassiveHost = __passiveNatWorkaroundStrategy.resolve(pasvHost);
|
|
+ if (!pasvHost.equals(newPassiveHost)) {
|
|
+ fireReplyReceived(0, "[Replacing PASV mode reply address " + __passiveHost + " with " + newPassiveHost + "]\n");
|
|
+ pasvHost = newPassiveHost;
|
|
+ }
|
|
+ } catch (final UnknownHostException e) { // Should not happen as we are passing in an IP address
|
|
+ throw new MalformedServerReplyException("Could not parse passive host information.\nServer Reply: " + reply);
|
|
}
|
|
- } catch (UnknownHostException e) { // Should not happen as we are passing in an IP address
|
|
- throw new MalformedServerReplyException(
|
|
- "Could not parse passive host information.\nServer Reply: " + reply);
|
|
}
|
|
+ } else if (_socket_ == null) {
|
|
+ pasvHost = null; // For unit testing.
|
|
+ } else {
|
|
+ pasvHost = _socket_.getInetAddress().getHostAddress();
|
|
}
|
|
+ __passiveHost = pasvHost;
|
|
+ __passivePort = pasvPort;
|
|
}
|
|
|
|
protected void _parseExtendedPassiveModeReply(String reply)
|
|
@@ -3968,6 +3988,43 @@ implements Configurable
|
|
}
|
|
return __systemName;
|
|
}
|
|
+
|
|
+ /**
|
|
+ * Returns, whether the IP address from the server's response should be used.
|
|
+ * Until 3.9.0, this has always been the case. Beginning with 3.9.0,
|
|
+ * that IP address will be silently ignored, and replaced with the remote
|
|
+ * IP address of the control connection, unless this configuration option is
|
|
+ * given, which restores the old behavior. To enable this by default, use
|
|
+ * the system property {@link FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE}.
|
|
+ * @return True, if the IP address from the server's response will be used
|
|
+ * (pre-3.9 compatible behavior), or false (ignore that IP address).
|
|
+ *
|
|
+ * @see FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE
|
|
+ * @see #setIpAddressFromPasvResponse(boolean)
|
|
+ * @since 3.9.0
|
|
+ */
|
|
+ public boolean isIpAddressFromPasvResponse() {
|
|
+ return ipAddressFromPasvResponse;
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * Sets whether the IP address from the server's response should be used.
|
|
+ * Until 3.9.0, this has always been the case. Beginning with 3.9.0,
|
|
+ * that IP address will be silently ignored, and replaced with the remote
|
|
+ * IP address of the control connection, unless this configuration option is
|
|
+ * given, which restores the old behavior. To enable this by default, use
|
|
+ * the system property {@link FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE}.
|
|
+ *
|
|
+ * @param usingIpAddressFromPasvResponse True, if the IP address from the
|
|
+ * server's response should be used (pre-3.9.0 compatible behavior), or
|
|
+ * false (ignore that IP address).
|
|
+ * @see FTPClient#FTP_IP_ADDRESS_FROM_PASV_RESPONSE
|
|
+ * @see #isIpAddressFromPasvResponse
|
|
+ * @since 3.9.0
|
|
+ */
|
|
+ public void setIpAddressFromPasvResponse(boolean usingIpAddressFromPasvResponse) {
|
|
+ this.ipAddressFromPasvResponse = usingIpAddressFromPasvResponse;
|
|
+ }
|
|
}
|
|
|
|
/* Emacs configuration
|
|
diff --git a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
|
|
index e43f075..faac5bd 100644
|
|
--- a/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
|
|
+++ b/src/test/java/org/apache/commons/net/ftp/FTPClientTest.java
|
|
@@ -147,56 +147,79 @@ public class FTPClientTest extends TestCase {
|
|
|
|
public void testParsePassiveModeReplyForLocalAddressWithNatWorkaround() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
assertEquals("8.8.8.8", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaround() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
assertEquals("8.8.4.4", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
@SuppressWarnings("deprecation") // testing deprecated code
|
|
public void testParsePassiveModeReplyForLocalAddressWithNatWorkaroundDisabled() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
client.setPassiveNatWorkaround(false);
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
assertEquals("172.16.204.138", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
@SuppressWarnings("deprecation") // testing deprecated code
|
|
public void testParsePassiveModeReplyForNonLocalAddressWithNatWorkaroundDisabled() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client.setPassiveNatWorkaround(false);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
assertEquals("8.8.4.4", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
public void testParsePassiveModeReplyForLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
client.setPassiveNatWorkaroundStrategy(null);
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
assertEquals("172.16.204.138", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
public void testParsePassiveModeReplyForNonLocalAddressWithoutNatWorkaroundStrategy() throws Exception {
|
|
FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client.setPassiveNatWorkaroundStrategy(null);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
assertEquals("8.8.4.4", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (8,8,4,4,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
|
|
public void testParsePassiveModeReplyForLocalAddressWithSimpleNatWorkaroundStrategy() throws Exception {
|
|
- FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
- client.setPassiveNatWorkaroundStrategy(new FTPClient.HostnameResolver() {
|
|
- @Override
|
|
- public String resolve(String hostname) throws UnknownHostException {
|
|
- return "4.4.4.4";
|
|
- }
|
|
-
|
|
- });
|
|
+ final FTPClient client = new PassiveNatWorkAroundLocalClient("8.8.8.8");
|
|
+ client.setPassiveNatWorkaroundStrategy(hostname -> "4.4.4.4");
|
|
+ client.setIpAddressFromPasvResponse(true);
|
|
client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
assertEquals("4.4.4.4", client.getPassiveHost());
|
|
+ client.setIpAddressFromPasvResponse(false);
|
|
+ client._parsePassiveModeReply("227 Entering Passive Mode (172,16,204,138,192,22).");
|
|
+ assertNull(client.getPassiveHost());
|
|
}
|
|
- }
|
|
+
|
|
+}
|