[FIXED] Check of maxpayload could be bypassed if size overruns int32

One could craft a PUB protocol to cause server to panic. This can
happen if the size in the PUB protocol overruns an int32.

(note that if authorization is enabled, the user would need to
authenticate first, limiting the impact).

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

Author: kozlovic

server/client.go

@@ -1567,11 +1567,13 @@ func (c *client) processPub(trace bool, arg []byte) error {
 	default:
 		return fmt.Errorf("processPub Parse Error: '%s'", arg)
 	}
+	// If number overruns an int64, parseSize() will have returned a negative value
 	if c.pa.size < 0 {
 		return fmt.Errorf("processPub Bad or Missing Size: '%s'", arg)
 	}
 	maxPayload := atomic.LoadInt32(&c.mpay)
-	if maxPayload != jwt.NoLimit && int32(c.pa.size) > maxPayload {
+	// Use int64() to avoid int32 overrun...
+	if maxPayload != jwt.NoLimit && int64(c.pa.size) > int64(maxPayload) {
 		c.maxPayloadViolation(c.pa.size, maxPayload)
 		return ErrMaxPayload
 	}

server/leafnode.go

@@ -1193,7 +1193,7 @@ func (c *client) processLeafMsgArgs(trace bool, arg []byte) error {
 		}
 	}
 	if c.pa.size < 0 {
-		return fmt.Errorf("processRoutedMsgArgs Bad or Missing Size: '%s'", args)
+		return fmt.Errorf("processLeafMsgArgs Bad or Missing Size: '%s'", args)
 	}
 
 	// Common ones processed after check for arg length

test/maxpayload_test.go

@@ -97,3 +97,28 @@ func TestMaxPayload(t *testing.T) {
 		}
 	}
 }
+
+func TestMaxPayloadOverrun(t *testing.T) {
+	opts := DefaultTestOptions
+	opts.Port = -1
+	opts.MaxPayload = 10000
+	s := RunServer(&opts)
+	defer s.Shutdown()
+
+	// Overrun a int32
+	c := createClientConn(t, "127.0.0.1", opts.Port)
+	defer c.Close()
+
+	send, expect := setupConn(t, c)
+	send("PUB foo 380571791000988\r\n")
+	expect(errRe)
+
+	// Now overrun an int64, parseSize will have returned -1,
+	// so we get disconnected.
+	c = createClientConn(t, "127.0.0.1", opts.Port)
+	defer c.Close()
+
+	send, _ = setupConn(t, c)
+	send("PUB foo 18446744073709551615123\r\n")
+	expectDisconnect(t, c)
+}