From 5d3984e353b0538b8672ca4e0ccecbbc94e94522 Mon Sep 17 00:00:00 2001 From: zzz <zzz@mail.i2p> Date: Wed, 26 Sep 2012 20:00:59 +0000 Subject: [PATCH] * Addresses: Reject numeric IPs of the form n, n.n, and n.n.n * Console, i2ptunnel: More validation of address and port in forms --- .../src/net/i2p/i2ptunnel/web/IndexBean.java | 29 ++++++++- apps/i2ptunnel/jsp/index.jsp | 10 +-- .../net/i2p/router/web/ConfigNetHandler.java | 62 ++++++++++++------- core/java/src/net/i2p/util/Addresses.java | 28 ++++++++- 4 files changed, 94 insertions(+), 35 deletions(-) diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/web/IndexBean.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/web/IndexBean.java index e4609593cf..28950f4476 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/web/IndexBean.java +++ b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/web/IndexBean.java @@ -32,6 +32,7 @@ import net.i2p.i2ptunnel.I2PTunnelHTTPClientBase; import net.i2p.i2ptunnel.I2PTunnelIRCClient; import net.i2p.i2ptunnel.TunnelController; import net.i2p.i2ptunnel.TunnelControllerGroup; +import net.i2p.util.Addresses; import net.i2p.util.ConcurrentHashSet; import net.i2p.util.FileUtil; import net.i2p.util.Log; @@ -436,6 +437,9 @@ public class IndexBean { return _("New Tunnel"); } + /** + * No validation + */ public String getClientPort(int tunnel) { TunnelController tun = getController(tunnel); if (tun != null && tun.getListenPort() != null) @@ -444,6 +448,23 @@ public class IndexBean { return ""; } + /** + * Returns error message if blank or invalid + * @since 0.9.3 + */ + public String getClientPort2(int tunnel) { + TunnelController tun = getController(tunnel); + if (tun != null && tun.getListenPort() != null) { + String port = tun.getListenPort(); + if (port.length() == 0) + return "<font color=\"red\">" + _("Port not set") + "</font>"; + if (Addresses.getPort(port) == 0) + return "<font color=\"red\">" + _("Invalid port") + ' ' + port + "</font>"; + return port; + } + return "<font color=\"red\">" + _("Port not set") + "</font>"; + } + public String getTunnelType(int tunnel) { TunnelController tun = getController(tunnel); if (tun != null) @@ -551,12 +572,16 @@ public class IndexBean { else host = tun.getTargetHost(); String port = tun.getTargetPort(); - if (host == null) + if (host == null || host.length() == 0) host = "<font color=\"red\">" + _("Host not set") + "</font>"; + else if (Addresses.getIP(host) == null) + host = "<font color=\"red\">" + _("Invalid address") + ' ' + host + "</font>"; else if (host.indexOf(':') >= 0) host = '[' + host + ']'; - if (port == null) + if (port == null || port.length() == 0) port = "<font color=\"red\">" + _("Port not set") + "</font>"; + else if (Addresses.getPort(port) == 0) + port = "<font color=\"red\">" + _("Invalid port") + ' ' + port + "</font>"; return host + ':' + port; } else return ""; diff --git a/apps/i2ptunnel/jsp/index.jsp b/apps/i2ptunnel/jsp/index.jsp index 5fad2c9ad4..6494430e4e 100644 --- a/apps/i2ptunnel/jsp/index.jsp +++ b/apps/i2ptunnel/jsp/index.jsp @@ -222,14 +222,8 @@ <label><%=intl._("Port")%>:</label> <span class="text"> <% - String cPort= indexBean.getClientPort(curClient); - if ("".equals(cPort)) { - out.write("<font color=\"red\">"); - out.write(intl._("Port not set")); - out.write("</font>"); - } else { - out.write(cPort); - } + String cPort= indexBean.getClientPort2(curClient); + out.write(cPort); %> </span> </div> diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java index a2298c7f71..da0fc328e2 100644 --- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java +++ b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java @@ -1,7 +1,5 @@ package net.i2p.router.web; -import java.net.InetAddress; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -13,6 +11,7 @@ import net.i2p.router.transport.TransportImpl; import net.i2p.router.transport.TransportManager; import net.i2p.router.transport.udp.UDPTransport; import net.i2p.router.web.ConfigServiceHandler; +import net.i2p.util.Addresses; /** * Handler to deal with form submissions from the main config form and act @@ -143,6 +142,7 @@ public class ConfigNetHandler extends FormHandler { */ private void saveChanges() { boolean restartRequired = false; + boolean error = false; List<String> removes = new ArrayList(); if (!_ratesOnly) { @@ -166,6 +166,8 @@ public class ConfigNetHandler extends FormHandler { valid = verifyAddress(uhost); if (valid) { changes.put(UDPTransport.PROP_EXTERNAL_HOST, uhost); + } else { + error = true; } } else { removes.add(UDPTransport.PROP_EXTERNAL_HOST); @@ -195,7 +197,9 @@ public class ConfigNetHandler extends FormHandler { valid = verifyAddress(_ntcpHostname); if (valid) { changes.put(ConfigNetHelper.PROP_I2NP_NTCP_HOSTNAME, _ntcpHostname); - addFormNotice(_("Updating inbound TCP address to") + " " + _ntcpHostname); + addFormNotice(_("Updating TCP address to {0}", _ntcpHostname)); + } else { + error = true; } } else { removes.add(ConfigNetHelper.PROP_I2NP_NTCP_HOSTNAME); @@ -212,8 +216,13 @@ public class ConfigNetHandler extends FormHandler { } if (oldAutoPort != _ntcpAutoPort || ! oldNPort.equals(_ntcpPort)) { if (_ntcpPort.length() > 0 && !_ntcpAutoPort) { - changes.put(ConfigNetHelper.PROP_I2NP_NTCP_PORT, _ntcpPort); - addFormNotice(_("Updating inbound TCP port to") + " " + _ntcpPort); + if (Addresses.getPort(_ntcpPort) != 0) { + changes.put(ConfigNetHelper.PROP_I2NP_NTCP_PORT, _ntcpPort); + addFormNotice(_("Updating TCP port to {0}", _ntcpPort)); + } else { + addFormError(_("Invalid port") + ": " + _ntcpPort); + error = true; + } } else { removes.add(ConfigNetHelper.PROP_I2NP_NTCP_PORT); addFormNotice(_("Updating inbound TCP port to auto")); @@ -226,10 +235,15 @@ public class ConfigNetHandler extends FormHandler { if ( (_udpPort != null) && (_udpPort.length() > 0) ) { String oldPort = _context.getProperty(UDPTransport.PROP_INTERNAL_PORT, "unset"); if (!oldPort.equals(_udpPort)) { - changes.put(UDPTransport.PROP_INTERNAL_PORT, _udpPort); - changes.put(UDPTransport.PROP_EXTERNAL_PORT, _udpPort); - addFormNotice(_("Updating UDP port from") + " " + oldPort + " " + _("to") + " " + _udpPort); - restartRequired = true; + if (Addresses.getPort(_udpPort) != 0) { + changes.put(UDPTransport.PROP_INTERNAL_PORT, _udpPort); + changes.put(UDPTransport.PROP_EXTERNAL_PORT, _udpPort); + addFormNotice(_("Updating UDP port to {0}", _udpPort)); + restartRequired = true; + } else { + addFormError(_("Invalid port") + ": " + _udpPort); + error = true; + } } } @@ -302,10 +316,11 @@ public class ConfigNetHandler extends FormHandler { if (ratesUpdated) _context.bandwidthLimiter().reinitialize(); - if (switchRequired) { - hiddenSwitch(); - } else if (restartRequired) { - //if (_context.hasWrapper()) { + if (saved && !error) { + if (switchRequired) { + hiddenSwitch(); + } else if (restartRequired) { + //if (_context.hasWrapper()) { // Wow this dumps all conns immediately and really isn't nice addFormNotice("Performing a soft restart"); _context.router().restart(); @@ -320,12 +335,13 @@ public class ConfigNetHandler extends FormHandler { // So don't do this... //_context.router().rebuildRouterInfo(); //addFormNotice("Router Info rebuilt"); - //} else { + //} else { // There's a few changes that don't really require restart (e.g. enabling inbound TCP) // But it would be hard to get right, so just do a restart. //addFormError(_("Gracefully restarting I2P to change published router address")); //_context.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART); - //} + //} + } } } @@ -337,17 +353,15 @@ public class ConfigNetHandler extends FormHandler { private boolean verifyAddress(String addr) { if (addr == null || addr.length() <= 0) return false; - try { - InetAddress ia = InetAddress.getByName(addr); - byte[] iab = ia.getAddress(); - boolean rv = TransportImpl.isPubliclyRoutable(iab); - if (!rv) - addFormError(_("The hostname or IP {0} is not publicly routable", addr)); - return rv; - } catch (UnknownHostException uhe) { - addFormError(_("The hostname or IP {0} is invalid", addr) + ": " + uhe); + byte[] iab = Addresses.getIP(addr); + if (iab == null) { + addFormError(_("Invalid address") + ": " + addr); return false; } + boolean rv = TransportImpl.isPubliclyRoutable(iab); + if (!rv) + addFormError(_("The hostname or IP {0} is not publicly routable", addr)); + return rv; } private void hiddenSwitch() { diff --git a/core/java/src/net/i2p/util/Addresses.java b/core/java/src/net/i2p/util/Addresses.java index c9c5a5e87c..9b29916a51 100644 --- a/core/java/src/net/i2p/util/Addresses.java +++ b/core/java/src/net/i2p/util/Addresses.java @@ -154,6 +154,26 @@ public abstract class Addresses { return "(bad IP length " + addr.length + "):" + port; } } + + /** + * Convenience method to convert and validate a port String + * without throwing an exception. + * Does not trim. + * + * @return 1-65535 or 0 if invalid + * @since 0.9.3 + */ + public static int getPort(String port) { + int rv = 0; + if (port != null) { + try { + int iport = Integer.parseInt(port); + if (iport > 0 && iport <= 65535) + rv = iport; + } catch (NumberFormatException nfe) {} + } + return rv; + } /** * Textual IP to bytes, because InetAddress.getByName() is slow. @@ -184,6 +204,9 @@ public abstract class Addresses { * Caches numeric host names only. * Will resolve but not cache DNS host names. * + * Unlike InetAddress.getByName(), we do NOT allow numeric IPs + * of the form d.d.d, d.d, or d, as these are almost certainly mistakes. + * * @param host DNS or IPv4 or IPv6 host name; if null returns null * @return IP or null * @since 0.9.3 @@ -197,8 +220,11 @@ public abstract class Addresses { } if (rv == null) { try { + boolean isIPv4 = host.replaceAll("[0-9\\.]", "").length() == 0; + if (isIPv4 && host.replaceAll("[0-9]", "").length() != 3) + return null; rv = InetAddress.getByName(host).getAddress(); - if (host.replaceAll("[0-9\\.]", "").length() == 0 || + if (isIPv4 || host.replaceAll("[0-9a-fA-F:]", "").length() == 0) { synchronized (_IPAddress) { _IPAddress.put(host, rv); -- GitLab