SOCKS: Fix NPE on lookup failure in SOCKS 4a

Remove duplicate lookups in SOCKS 5
Log tweaks
Streaming, i2ptunnel: Catch null destination in connect() calls
Synchronize Connection.setRemotePeer(); remove _remotePeerSet field

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClientBase.java

@@ -626,7 +626,7 @@ public abstract class I2PTunnelClientBase extends I2PTunnelTask implements Runna
      * adding it to the list of connections actually managed by this
      * tunnel.
      *
-     * @param dest The destination to connect to
+     * @param dest The destination to connect to, non-null
      * @return a new I2PSocket
      */
     public I2PSocket createI2PSocket(Destination dest) throws I2PException, ConnectException, NoRouteToHostException, InterruptedIOException {
@@ -638,7 +638,7 @@ public abstract class I2PTunnelClientBase extends I2PTunnelTask implements Runna
      * adding it to the list of connections actually managed by this
      * tunnel.
      *
-     * @param dest The destination to connect to
+     * @param dest The destination to connect to, non-null
      * @param port The destination port to connect to 0 - 65535
      * @return a new I2PSocket
      * @since 0.9.9
@@ -656,7 +656,7 @@ public abstract class I2PTunnelClientBase extends I2PTunnelTask implements Runna
      * adding it to the list of connections actually managed by this
      * tunnel.
      *
-     * @param dest The destination to connect to
+     * @param dest The destination to connect to, non-null
      * @param opt Option to be used to open when opening the socket
      * @return a new I2PSocket
      *
@@ -666,6 +666,8 @@ public abstract class I2PTunnelClientBase extends I2PTunnelTask implements Runna
      * @throws I2PException if there is some other I2P-related problem
      */
     public I2PSocket createI2PSocket(Destination dest, I2PSocketOptions opt) throws I2PException, ConnectException, NoRouteToHostException, InterruptedIOException {
+        if (dest == null)
+            throw new NullPointerException();
         I2PSocket i2ps;
 
         verifySocketManager();

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/socks/SOCKS4aServer.java

@@ -22,6 +22,7 @@ import net.i2p.I2PException;
 import net.i2p.client.streaming.I2PSocket;
 import net.i2p.client.streaming.I2PSocketOptions;
 import net.i2p.data.DataFormatException;
+import net.i2p.data.Destination;
 import net.i2p.util.HexDump;
 import net.i2p.util.Log;
 
@@ -212,14 +213,22 @@ public class SOCKS4aServer extends SOCKSServer {
         try {
             if (connHostName.toLowerCase(Locale.US).endsWith(".i2p") ||
                 connHostName.toLowerCase(Locale.US).endsWith(".onion")) {
-                _log.debug("connecting to " + connHostName + "...");
+                // Let's not do a new Dest for every request, huh?
-                // Let's not due a new Dest for every request, huh?
                 //I2PSocketManager sm = I2PSocketManagerFactory.createManager();
                 //destSock = sm.connect(I2PTunnel.destFromName(connHostName), null);
+                Destination dest = I2PAppContext.getGlobalContext().namingService().lookup(connHostName);
+                if (dest == null) {
+                    try {
+                        sendRequestReply(Reply.CONNECTION_REFUSED, InetAddress.getByName("127.0.0.1"), 0, out);
+                    } catch (IOException ioe) {}
+                    throw new SOCKSException("Host not found");
+                }
+                if (_log.shouldDebug())
+                    _log.debug("connecting to " + connHostName + "...");
                 Properties overrides = new Properties();
                 I2PSocketOptions sktOpts = t.buildOptions(overrides);
                 sktOpts.setPort(connPort);
-                destSock = t.createI2PSocket(I2PAppContext.getGlobalContext().namingService().lookup(connHostName), sktOpts);
+                destSock = t.createI2PSocket(dest, sktOpts);
             } else if ("localhost".equals(connHostName) || "127.0.0.1".equals(connHostName)) {
                 String err = "No localhost accesses allowed through the Socks Proxy";
                 _log.error(err);
@@ -249,10 +258,18 @@ public class SOCKS4aServer extends SOCKSServer {
                 }
                 int p = I2PAppContext.getGlobalContext().random().nextInt(proxies.size());
                 String proxy = proxies.get(p);
-                _log.debug("connecting to port " + connPort + " proxy " + proxy + " for " + connHostName + "...");
+                Destination dest = I2PAppContext.getGlobalContext().namingService().lookup(proxy);
+                if (dest == null) {
+                    try {
+                        sendRequestReply(Reply.CONNECTION_REFUSED, InetAddress.getByName("127.0.0.1"), 0, out);
+                    } catch (IOException ioe) {}
+                    throw new SOCKSException("Outproxy not found");
+                }
+                if (_log.shouldDebug())
+                    _log.debug("connecting to port " + connPort + " proxy " + proxy + " for " + connHostName + "...");
                 // this isn't going to work, these need to be socks outproxies so we need
                 // to do a socks session to them?
-                destSock = t.createI2PSocket(I2PAppContext.getGlobalContext().namingService().lookup(proxy));
+                destSock = t.createI2PSocket(dest);
             }
             confirmConnection();
             _log.debug("connection confirmed - exchanging data...");

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/socks/SOCKS5Server.java

@@ -363,8 +363,7 @@ public class SOCKS5Server extends SOCKSServer {
 
         try {
             if (connHostName.toLowerCase(Locale.US).endsWith(".i2p")) {
-                _log.debug("connecting to " + connHostName + "...");
+                // Let's not do a new Dest for every request, huh?
-                // Let's not due a new Dest for every request, huh?
                 //I2PSocketManager sm = I2PSocketManagerFactory.createManager();
                 //destSock = sm.connect(I2PTunnel.destFromName(connHostName), null);
                 Destination dest = I2PAppContext.getGlobalContext().namingService().lookup(connHostName);
@@ -374,10 +373,12 @@ public class SOCKS5Server extends SOCKSServer {
                     } catch (IOException ioe) {}
                     throw new SOCKSException("Host not found");
                 }
+                if (_log.shouldDebug())
+                    _log.debug("connecting to " + connHostName + "...");
                 Properties overrides = new Properties();
                 I2PSocketOptions sktOpts = t.buildOptions(overrides);
                 sktOpts.setPort(connPort);
-                destSock = t.createI2PSocket(I2PAppContext.getGlobalContext().namingService().lookup(connHostName), sktOpts);
+                destSock = t.createI2PSocket(dest, sktOpts);
             } else if ("localhost".equals(connHostName) || "127.0.0.1".equals(connHostName)) {
                 String err = "No localhost accesses allowed through the Socks Proxy";
                 _log.error(err);
@@ -468,7 +469,7 @@ public class SOCKS5Server extends SOCKSServer {
         Destination dest = I2PAppContext.getGlobalContext().namingService().lookup(proxy);
         if (dest == null)
             throw new SOCKSException("Outproxy not found");
-        I2PSocket destSock = tun.createI2PSocket(I2PAppContext.getGlobalContext().namingService().lookup(proxy), proxyOpts);
+        I2PSocket destSock = tun.createI2PSocket(dest, proxyOpts);
         DataOutputStream out = null;
         DataInputStream in = null;
         try {

apps/streaming/java/src/net/i2p/client/streaming/impl/Connection.java

@@ -826,15 +826,22 @@ class Connection {
         _timer.addEvent(event, msToWait);
     }
 
-    private boolean _remotePeerSet = false;
     /** who are we talking with
-     * @return peer Destination
+     * @return peer Destination or null if unset
+     */
+    public synchronized Destination getRemotePeer() { return _remotePeer; }
+
+    /**
+     *  @param peer non-null
      */
-    public Destination getRemotePeer() { return _remotePeer; }
     public void setRemotePeer(Destination peer) { 
-        if (_remotePeerSet) throw new RuntimeException("Remote peer already set [" + _remotePeer + ", " + peer + "]");
+        if (peer == null)
-        _remotePeerSet = true;
+            throw new NullPointerException();
-        _remotePeer = peer; 
+        synchronized(this) {
+            if (_remotePeer != null)
+                throw new RuntimeException("Remote peer already set [" + _remotePeer + ", " + peer + "]");
+            _remotePeer = peer; 
+        }
         // now that we know who the other end is, get the rtt etc. from the cache
         _connectionManager.updateOptsFromShare(this);
     }
@@ -1220,7 +1227,7 @@ class Connection {
             buf.append(" from ");
         else
             buf.append(" to ");
-        if (_remotePeerSet)
+        if (_remotePeer != null)
             buf.append(_remotePeer.calculateHash().toBase64().substring(0,4));
         else
             buf.append("unknown");

apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionManager.java

@@ -395,12 +395,14 @@ class ConnectionManager {
      * Build a new connection to the given peer.  This blocks if there is no
      * connection delay, otherwise it returns immediately.
      *
-     * @param peer Destination to contact
+     * @param peer Destination to contact, non-null
      * @param opts Connection's options
      * @param session generally the session from the constructor, but could be a subsession
      * @return new connection, or null if we have exceeded our limit
      */
     public Connection connect(Destination peer, ConnectionOptions opts, I2PSession session) {
+        if (peer == null)
+            throw new NullPointerException();
         Connection con = null;
         long expiration = _context.clock().now();
         long tmout = opts.getConnectTimeout();

apps/streaming/java/src/net/i2p/client/streaming/impl/ConnectionPacketHandler.java

@@ -479,7 +479,13 @@ class ConnectionPacketHandler {
             if (con.getSendStreamId() <= 0) {
                 if (packet.isFlagSet(Packet.FLAG_SYNCHRONIZE)) {
                     con.setSendStreamId(packet.getReceiveStreamId());
-                    con.setRemotePeer(packet.getOptionalFrom());
+                    Destination dest = packet.getOptionalFrom();
+                    if (dest == null) {
+                        if (_log.shouldWarn())
+                            _log.warn("SYN Packet without FROM");
+                        return false;
+                    }
+                    con.setRemotePeer(dest);
                     return true;
                 } else {
                     // neither RST nor SYN and we dont have the stream id yet?

apps/streaming/java/src/net/i2p/client/streaming/impl/I2PSocketManagerFull.java

@@ -524,6 +524,8 @@ public class I2PSocketManagerFull implements I2PSocketManager {
      */
     public I2PSocket connect(Destination peer, I2PSocketOptions options) 
                              throws I2PException, NoRouteToHostException {
+        if (peer == null)
+            throw new NullPointerException();
         if (options == null)
             options = _defaultOptions;
         ConnectionOptions opts = null;

history.txt

@@ -1,3 +1,6 @@
+2016-04-13 zzz
+ * SOCKS: Fix NPE on lookup failure in SOCKS 4a
+
 2016-04-07 zzz
  * Debian builds:
    - Prep for depending on tomcat packages

router/java/src/net/i2p/router/RouterVersion.java

@@ -18,7 +18,7 @@ public class RouterVersion {
     /** deprecated */
     public final static String ID = "Monotone";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 1;
+    public final static long BUILD = 2;
 
     /** for example "-test" */
     public final static String EXTRA = "";