[tarantool-patches] [PATCH v2] sql: display decoded msgpack for EXPLAIN queries

[tarantool-patches] [PATCH v2] sql: display decoded msgpack for EXPLAIN queries

[Nikita Pettik]

During DDL routines we pass encoded space/index/trigger formats
into msgpack to VDBE. EXPLAIN query displays arguments of each opcode of
VDBE program in a readable format. So, lets decode arguments of OP_Blob
opcode with subtype = _MSGPACK before displaying them. Also, lets
enlarge static buffers for P4 operand value and opcode comment to fit
decoded msgpack.

What is more, it fixes buffer-overflow since before this patch operands
of OP_Blob were treated as strings and passed to functions like strlen()
(only during EXPLAIN query). On the other hand, generally speaking
msgpack can come without null termination, or contain '\0' symbols in
the middle of encoded array.

Closes #3868
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3868-buffer-overflow-v2
Issue: https://github.com/tarantool/tarantool/issues/3868

Discussion of previous version:
https://www.freelists.org/post/tarantool-patches/PATCH-sql-terminate-with-0-encoded-msgpack

 src/box/sql/vdbeaux.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index b831b52ad..30fb5398a 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1284,6 +1284,15 @@ displayComment(const Op * pOp,	/* The opcode to be commented */
 static char *
 displayP4(Op * pOp, char *zTemp, int nTemp)
 {
+	/*
+	 * Msgpack is subtype, not type of P4, so lets consider
+	 * it as special case. We should decode msgpack to display
+	 * it in a readable form.
+	 */
+	if (pOp->opcode == OP_Blob && pOp->p3 == SQL_SUBTYPE_MSGPACK) {
+		mp_snprint(zTemp, nTemp, pOp->p4.z);
+		return zTemp;
+	}
 	char *zP4 = zTemp;
 	StrAccum x;
 	assert(nTemp >= 20);
@@ -1416,8 +1425,8 @@ void
 sqlVdbePrintOp(FILE * pOut, int pc, Op * pOp)
 {
 	char *zP4;
-	char zPtr[50];
-	char zCom[100];
+	char zPtr[256];
+	char zCom[256];
 	static const char *zFormat1 =
 	    "%4d> %4d %-13s %4d %4d %4d %-13s %.2X %s\n";
 	if (pOut == 0)
@@ -1674,12 +1683,13 @@ sqlVdbeList(Vdbe * p)
 		pMem->u.i = pOp->p3;	/* P3 */
 		pMem++;
 
-		if (sqlVdbeMemClearAndResize(pMem, 100)) {	/* P4 */
+		if (sqlVdbeMemClearAndResize(pMem, 256)) {
 			assert(p->db->mallocFailed);
 			return SQL_ERROR;
 		}
 		pMem->flags = MEM_Str | MEM_Term;
 		zP4 = displayP4(pOp, pMem->z, pMem->szMalloc);
 		if (zP4 != pMem->z) {
 			pMem->n = 0;
 			sqlVdbeMemSetStr(pMem, zP4, -1, 1, 0);
-- 
2.15.1

[tarantool-patches] Re: [PATCH v2] sql: display decoded msgpack for EXPLAIN queries

[Vladislav Shpilevoy]

On 15/02/2019 20:37, Nikita Pettik wrote:
> During DDL routines we pass encoded space/index/trigger formats
> into msgpack to VDBE. EXPLAIN query displays arguments of each opcode of
> VDBE program in a readable format. So, lets decode arguments of OP_Blob
> opcode with subtype = _MSGPACK before displaying them. Also, lets
> enlarge static buffers for P4 operand value and opcode comment to fit
> decoded msgpack.
> 
> What is more, it fixes buffer-overflow since before this patch operands
> of OP_Blob were treated as strings and passed to functions like strlen()
> (only during EXPLAIN query). On the other hand, generally speaking
> msgpack can come without null termination, or contain '\0' symbols in
> the middle of encoded array.
> 
> Closes #3868
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3868-buffer-overflow-v2
> Issue: https://github.com/tarantool/tarantool/issues/3868
> 

LGTM.

[tarantool-patches] Re: [PATCH v2] sql: display decoded msgpack for EXPLAIN queries

[Kirill Yukhin]

Hello,

On 15 Feb 20:37, Nikita Pettik wrote:
> During DDL routines we pass encoded space/index/trigger formats
> into msgpack to VDBE. EXPLAIN query displays arguments of each opcode of
> VDBE program in a readable format. So, lets decode arguments of OP_Blob
> opcode with subtype = _MSGPACK before displaying them. Also, lets
> enlarge static buffers for P4 operand value and opcode comment to fit
> decoded msgpack.
> 
> What is more, it fixes buffer-overflow since before this patch operands
> of OP_Blob were treated as strings and passed to functions like strlen()
> (only during EXPLAIN query). On the other hand, generally speaking
> msgpack can come without null termination, or contain '\0' symbols in
> the middle of encoded array.
> 
> Closes #3868
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3868-buffer-overflow-v2
> Issue: https://github.com/tarantool/tarantool/issues/3868

Your patch was checked into 2.1 branch few days ago.

--
Regards, Kirill Yukhin