From 53fd02919be4702ee2462c38fc7e226708e7706e Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan@synadia.com>
Date: Mon, 18 Nov 2019 14:46:09 -0700
Subject: [PATCH] Added split test buffer that shows the subject overwritte

With this test, and with previous cloneArg() code, the parsing
will cause subject to become "fff" because c.pa.subject points
to somewhere in the underlying buffer that is now overwritten
with the body payload. With proper cloneArg(), the c.pa.subject
(and other) point to the copy of argBuf.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
---
 server/parser.go     |  2 +-
 server/split_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/server/parser.go b/server/parser.go
index 3c129977..72eb293f 100644
--- a/server/parser.go
+++ b/server/parser.go
@@ -282,7 +282,7 @@ func (c *client) parse(buf []byte) error {
 			c.drop, c.as, c.state = 0, i+1, OP_START
 			// Drop all pub args
 			c.pa.arg, c.pa.pacache, c.pa.account, c.pa.subject = nil, nil, nil, nil
-			c.pa.reply, c.pa.szb, c.pa.queues = nil, nil, nil
+			c.pa.reply, c.pa.size, c.pa.szb, c.pa.queues = nil, 0, nil, nil
 		case OP_A:
 			switch b {
 			case '+':
diff --git a/server/split_test.go b/server/split_test.go
index a5e625eb..77c726c2 100644
--- a/server/split_test.go
+++ b/server/split_test.go
@@ -515,3 +515,53 @@ func TestSplitBufferMsgOp(t *testing.T) {
 		t.Fatalf("Expected MSG_END_N state vs %d\n", c.state)
 	}
 }
+
+func TestSplitBufferLeafMsgArg(t *testing.T) {
+	c := &client{msubs: -1, mpay: -1, mcl: 1024, subs: make(map[string]*subscription), kind: LEAF}
+	msg := []byte("LMSG foo + bar baz 11\r\n")
+	if err := c.parse(msg); err != nil {
+		t.Fatalf("Unexpected parse error: %v\n", err)
+	}
+	if c.state != MSG_PAYLOAD {
+		t.Fatalf("Expected MSG_PAYLOAD state vs %d\n", c.state)
+	}
+	checkPA := func(t *testing.T) {
+		t.Helper()
+		if !bytes.Equal(c.pa.subject, []byte("foo")) {
+			t.Fatalf("Expected subject to be %q, got %q", "foo", c.pa.subject)
+		}
+		if !bytes.Equal(c.pa.reply, []byte("bar")) {
+			t.Fatalf("Expected reply to be %q, got %q", "bar", c.pa.reply)
+		}
+		if n := len(c.pa.queues); n != 1 {
+			t.Fatalf("Expected 1 queue, got %v", n)
+		}
+		if !bytes.Equal(c.pa.queues[0], []byte("baz")) {
+			t.Fatalf("Expected queues to be %q, got %q", "baz", c.pa.queues)
+		}
+	}
+	checkPA(t)
+
+	// overwrite msg with payload
+	n := copy(msg, []byte("fffffffffff"))
+	if err := c.parse(msg[:n]); err != nil {
+		t.Fatalf("Unexpected parse error: %v\n", err)
+	}
+	if c.state != MSG_END_R {
+		t.Fatalf("Expected MSG_END_R state vs %d\n", c.state)
+	}
+	checkPA(t)
+
+	// Finish processing
+	copy(msg, []byte("\r\n"))
+	if err := c.parse(msg[:2]); err != nil {
+		t.Fatalf("Unexpected parse error: %v\n", err)
+	}
+	if c.state != OP_START {
+		t.Fatalf("Expected OP_START state vs %d\n", c.state)
+	}
+	if c.pa.subject != nil || c.pa.reply != nil || c.pa.queues != nil || c.pa.size != 0 ||
+		c.pa.szb != nil || c.pa.arg != nil {
+		t.Fatalf("parser state not cleaned-up properly: %+v", c.pa)
+	}
+}