[tarantool-patches] [PATCH v1 1/1] lua_cjson: fix segfault on recursive table encoding

[tarantool-patches] [PATCH v1 1/1] lua_cjson: fix segfault on recursive table encoding

[Kirill Shcherbatov]

The json.encode() used to cause a segfault in case of recursive
table:
  tbl = {}
  tbl[1] = tbl
  json.encode(tbl)

Library doesn't test whether given object on Lua stack parsed
earlier, because it performs a lightweight in-depth traverse
of Lua stack. However it must stop when encode_max_depth is
reached (by design).

Tarantool's lua_cjson implementation has a bug introduced during
porting original library: it doesn't handle some corner cases
checking this constant while original code doesn't have such
problem. This patch adopts author's approach to check
encode_max_depth limit. Thanks to handling this constraint
correctly the segfault no longer occurs.

Closes #4366
---
Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4366-json-recursive-table-segfault
Issue: https://github.com/tarantool/tarantool/issues/4366

 third_party/lua-cjson/lua_cjson.c |  6 +++---
 test/app-tap/json.test.lua        | 23 ++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index 631c53e3f..be2416c69 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -364,7 +364,7 @@ static void json_append_object(lua_State *l, struct luaL_serializer *cfg,
     }
 
         /* table, key, value */
-    json_append_data(l, cfg, current_depth + 1, json);
+        json_append_data(l, cfg, current_depth, json);
         lua_pop(l, 1);
         /* table, key */
     }
@@ -402,13 +402,13 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
     case MP_MAP:
     if (current_depth >= cfg->encode_max_depth)
         return json_append_nil(cfg, json); /* Limit nested maps */
-    json_append_object(l, cfg, current_depth, json);
+    json_append_object(l, cfg, current_depth + 1, json);
     return;
     case MP_ARRAY:
     /* Array */
     if (current_depth >= cfg->encode_max_depth)
         return json_append_nil(cfg, json); /* Limit nested arrays */
-    json_append_array(l, cfg, current_depth, json, field.size);
+    json_append_array(l, cfg, current_depth + 1, json, field.size);
     return;
     case MP_EXT:
     /* handled by luaL_convertfield */
diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua
index 7ec3b11c7..10f6f4ab2 100755
--- a/test/app-tap/json.test.lua
+++ b/test/app-tap/json.test.lua
@@ -22,7 +22,7 @@ end
 
 tap.test("json", function(test)
     local serializer = require('json')
-    test:plan(28)
+    test:plan(32)
 
     test:test("unsigned", common.test_unsigned, serializer)
     test:test("signed", common.test_signed, serializer)
@@ -100,4 +100,25 @@ tap.test("json", function(test)
     test:is(serializer.decode('{"var":2.0e+3}')["var"], 2000)
     test:is(serializer.decode('{"var":2.0e+3}')["var"], 2000)
     test:is(serializer.decode('{"var":2.0e+3}')["var"], 2000)
+
+    --
+    -- gh-4366: segmentation fault with recursive table
+    --
+    serializer.cfg({encode_max_depth = 2})
+    local rec1 = {}
+    rec1[1] = rec1
+    test:is(serializer.encode(rec1), '[[null]]')
+    local rec2 = {}
+    rec2['x'] = rec2
+    test:is(serializer.encode(rec2), '{"x":{"x":null}}')
+    local rec3 = {}
+    rec3[1] = rec3
+    rec3[2] = rec3
+    test:is(serializer.encode(rec3), '[[null,null],[null,null]]')
+    local rec4 = {}
+    rec4['a'] = rec4
+    rec4['b'] = rec4
+    test:is(serializer.encode(rec4),
+            '{"a":{"a":null,"b":null},"b":{"a":null,"b":null}}')
+    serializer.cfg({encode_max_depth = orig_encode_max_depth})
 end)
-- 
2.22.1

[tarantool-patches] Re: [PATCH v1 1/1] lua_cjson: fix segfault on recursive table encoding

[Kirill Yukhin]

Hello,

On 19 Aug 15:37, Kirill Shcherbatov wrote:
> The json.encode() used to cause a segfault in case of recursive
> table:
>   tbl = {}
>   tbl[1] = tbl
>   json.encode(tbl)
> 
> Library doesn't test whether given object on Lua stack parsed
> earlier, because it performs a lightweight in-depth traverse
> of Lua stack. However it must stop when encode_max_depth is
> reached (by design).
> 
> Tarantool's lua_cjson implementation has a bug introduced during
> porting original library: it doesn't handle some corner cases
> checking this constant while original code doesn't have such
> problem. This patch adopts author's approach to check
> encode_max_depth limit. Thanks to handling this constraint
> correctly the segfault no longer occurs.
> 
> Closes #4366
> ---
> Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4366-json-recursive-table-segfault
> Issue: https://github.com/tarantool/tarantool/issues/4366

I've checked your patch into 1.10, 2.1, 2.2 and master.

--
Regards, Kirill Yukhin