* Addresses: Reject numeric IPs of the form n, n.n, and n.n.n

* Console, i2ptunnel: More validation of address and port in forms

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 "";

apps/i2ptunnel/jsp/index.jsp

@@ -222,14 +222,8 @@
             <label><%=intl._("Port")%>:</label>
             <span class="text">
          <%
-               String cPort= indexBean.getClientPort(curClient);
+               String cPort= indexBean.getClientPort2(curClient);
-               if ("".equals(cPort)) {
+               out.write(cPort);
-                   out.write("<font color=\"red\">");
-                   out.write(intl._("Port not set"));
-                   out.write("</font>");
-               } else {
-                   out.write(cPort);
-               }
           %>
             </span>
         </div>

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);
+                    if (Addresses.getPort(_ntcpPort) != 0) {
-                    addFormNotice(_("Updating inbound TCP port to") + " " + _ntcpPort);
+                        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);
+                    if (Addresses.getPort(_udpPort) != 0) {
-                    changes.put(UDPTransport.PROP_EXTERNAL_PORT, _udpPort);
+                        changes.put(UDPTransport.PROP_INTERNAL_PORT, _udpPort);
-                    addFormNotice(_("Updating UDP port from") + " " + oldPort + " " + _("to") + " " + _udpPort);
+                        changes.put(UDPTransport.PROP_EXTERNAL_PORT, _udpPort);
-                    restartRequired = true;
+                        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) {
+        if (saved && !error) {
-            hiddenSwitch();
+            if (switchRequired) {
-        } else if (restartRequired) {
+                hiddenSwitch();
-            //if (_context.hasWrapper()) {
+            } 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 {
+        byte[] iab = Addresses.getIP(addr);
-            InetAddress ia = InetAddress.getByName(addr);
+        if (iab == null) {
-            byte[] iab = ia.getAddress();
+            addFormError(_("Invalid address") + ": " + addr);
-            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);
             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() {

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);