From: Markus Koschany 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)}.
@@ -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. *

@@ -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()); } - } + +}