Changeset 18e24ed

Timestamp:

Jul 23, 2018 8:50:42 PM (21 months ago)

Author:

zzz <zzz@…>

Branches:

master

Children:

2893cbb, fe41dec

Parents:

f554ca3

Message:

NTCP2: Fix double-free of buffers after msg3 p2 fails
Fix sending termination after msg3 p2 fails

Location:

router/java/src/net/i2p/router

Files:

• RouterVersion.java (modified) (1 diff)
• transport/ntcp/InboundEstablishState.java (modified) (9 diffs)

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

@@ -19 +19 @@
     public final static String ID = "Monotone";
     public final static String VERSION = CoreVersion.VERSION;
-    public final static long BUILD = 16;
+    public final static long BUILD = 17;
 
     /** for example "-test" */

router/java/src/net/i2p/router/transport/ntcp/InboundEstablishState.java

@@ -57 +57 @@
     /** how long we expect _sz_aliceIdent_tsA_padding_aliceSig to be when its full */
     private int _sz_aliceIdent_tsA_padding_aliceSigSize;
+
+    private boolean _released;
 
     //// NTCP2 things
@@ -519 +521 @@
             if(ip != null)
                _context.blocklist().add(ip);
-            fail("Peer is banlisted forever: " + aliceHash);
+            if (getVersion() < 2)
+                fail("Peer is banlisted forever: " + aliceHash);
+            else if (_log.shouldWarn())
+                _log.warn("Peer is banlisted forever: " + aliceHash);
             _msg3p2FailReason = NTCPConnection.REASON_BANNED;
             return false;
@@ -545 +550 @@
                                                _x("Excessive clock skew: {0}"));
             _transport.setLastBadSkew(_peerSkew);
-            fail("Clocks too skewed (" + diff + " ms)", null, true);
+            if (getVersion() < 2)
+                fail("Clocks too skewed (" + diff + " ms)", null, true);
+            else if (_log.shouldWarn())
+                _log.warn("Clocks too skewed (" + diff + " ms)");
             _msg3p2FailReason = NTCPConnection.REASON_SKEW;
             return false;
@@ -785 +793 @@
                 NTCP2Payload.processPayload(_context, this, payload, 0, _msg3p2len - MAC_SIZE, true);
             } catch (IOException ioe) {
-                fail("Bad msg 3 payload", ioe);
+                if (_log.shouldWarn())
+                    _log.warn("Bad msg 3 payload", ioe);
                 // probably payload frame/block problems
                 // setDataPhase() will send termination
@@ -791 +800 @@
                     _msg3p2FailReason = NTCPConnection.REASON_FRAMING;
             } catch (DataFormatException dfe) {
-                fail("Bad msg 3 payload", dfe);
+                if (_log.shouldWarn())
+                    _log.warn("Bad msg 3 payload", dfe);
                 // probably RI problems
                 // setDataPhase() will send termination
@@ -799 +809 @@
             } catch (I2NPMessageException ime) {
                 // shouldn't happen, no I2NP msgs in msg3p2
-                fail("Bad msg 3 payload", ime);
+                if (_log.shouldWarn())
+                    _log.warn("Bad msg 3 payload", ime);
                 // setDataPhase() will send termination
                 if (_msg3p2FailReason < 0)
@@ -859 +870 @@
 
     /**
-     *  KDF for NTCP2 data phase,
-     *  then calls con.finishInboundEstablishment(),
-     *  passing over the final keys and states to the con.
-     *
-     *  This changes the state to VERIFIED.
+     *  KDF for NTCP2 data phase.
+     *
+     *  If _msg3p2FailReason is less than zero,
+     *  this calls con.finishInboundEstablishment(),
+     *  passing over the final keys and states to the con,
+     *  and changes the state to VERIFIED.
+     *
+     *  Otherwise, it calls con.failInboundEstablishment(),
+     *  which will send a termination message,
+     *  and changes the state to CORRUPT.
+     *
+     *  If you don't call this, call fail().
      *
      *  @param buf possibly containing "extra" data for data phase
@@ -886 +904 @@
                 _log.warn("Failed msg3p2, code " + _msg3p2FailReason + " for " + this);
             _con.failInboundEstablishment(sender, sip_ba, _msg3p2FailReason);
+            changeState(State.CORRUPT);
         } else {
             if (_log.shouldDebug()) {
@@ -1032 +1051 @@
     @Override
     protected void releaseBufs(boolean isVerified) {
+        if (_released)
+            return;
+        _released = true;
         super.releaseBufs(isVerified);
         // Do not release _curEncrypted if verified, it is passed to