From: Petr Štetiar <ynezz@true.cz>
Date: Mon, 4 Nov 2019 22:08:26 +0000 (+0100)
Subject: lua: fix memory leak in set method
X-Git-Url: http://git.cdn.openwrt.org/?a=commitdiff_plain;h=39093f3b040df895da812e2932b55feeeb1201df;p=project%2Fuci.git

lua: fix memory leak in set method

scan-build from clang version 9 has reported following issue:

 uci.c:624:12: warning: Potential leak of memory pointed to by 's'
       return luaL_error(L, "Cannot set an uci option to an empty table value");
              ^~~~~~~~~~

valgrind confirmed it on the supplied test case:

 ==31013== 8 bytes in 1 blocks are definitely lost in loss record 1 of 1
 ==31013==    by 0x56C49B9: strdup (strdup.c:42)

Signed-off-by: Petr Štetiar <ynezz@true.cz>
---

diff --git a/lua/uci.c b/lua/uci.c
index 42fe4b7..ad08299 100644
--- a/lua/uci.c
+++ b/lua/uci.c
@@ -620,8 +620,10 @@ uci_lua_set(lua_State *L)
 	case 4:
 		/* Format: uci.set("p", "s", "o", "v") */
 		if (lua_istable(L, nargs)) {
-			if (lua_rawlen(L, nargs) < 1)
+			if (lua_rawlen(L, nargs) < 1) {
+				free(s);
 				return luaL_error(L, "Cannot set an uci option to an empty table value");
+			}
 			lua_rawgeti(L, nargs, 1);
 			ptr.value = luaL_checkstring(L, -1);
 			lua_pop(L, 1);
diff --git a/tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua b/tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua
new file mode 100644
index 0000000..83e91d3
--- /dev/null
+++ b/tests/cram/lua/test_cases/set_with_empty_table_doesnt_leak.lua
@@ -0,0 +1,2 @@
+local c = uci.cursor(os.getenv("CONFIG_DIR"))
+print(pcall(c.set, c, "network", "lan", "dns", {}))
diff --git a/tests/cram/test_ucilua_testcases.t b/tests/cram/test_ucilua_testcases.t
index 279dfce..6a0216b 100644
--- a/tests/cram/test_ucilua_testcases.t
+++ b/tests/cram/test_ucilua_testcases.t
@@ -8,3 +8,10 @@ check that changes method doesnt leak memory:
   $ cp -R "$TESTDIR/config" .
   $ export CONFIG_DIR=$(pwd)/config
   $ ucilua $TESTDIR/lua/test_cases/changes_doesnt_leak.lua
+
+check that set method with empty table value doesn't leak memory:
+
+  $ cp -R "$TESTDIR/config" .
+  $ export CONFIG_DIR=$(pwd)/config
+  $ ucilua $TESTDIR/lua/test_cases/set_with_empty_table_doesnt_leak.lua
+  false\tCannot set an uci option to an empty table value (esc)