Fix assorted minor memory management issues

owasp-modsecurity · Nov 14, 2017 · 9c0ed61 · 9c0ed61
1 parent b7698d6
commit 9c0ed61
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 10 deletions.
diff --git a/src/engine/lua.cc b/src/engine/lua.cc
@@ -122,6 +122,8 @@ const char *Lua::blob_reader(lua_State *L, void *ud, size_t *size) {
 int Lua::run(Transaction *t) {
 #ifdef WITH_LUA
     std::string luaRet;
+    const char *a = NULL;
+    int ret = true;
     lua_State *L = luaL_newstate();
     luaL_openlibs(L);
 
@@ -152,7 +154,8 @@ int Lua::run(Transaction *t) {
         }
         e.append(lua_tostring(L, -1));
         t->debug(2, e);
-        return false;
+        ret = false;
+        goto err;
     }
 
     if (lua_pcall(L, 0, 0, 0)) {
@@ -165,7 +168,8 @@ int Lua::run(Transaction *t) {
             e.append(luaerr);
         }
         t->debug(2, e);
-        return false;
+        ret = false;
+        goto err;
     }
 
     lua_setglobal(L, "modsec");
@@ -180,24 +184,27 @@ int Lua::run(Transaction *t) {
             e.append(luaerr);
         }
         t->debug(2, e);
-        return false;
+        ret = false;
+        goto err;
     }
 
-    const char *a = reinterpret_cast<const char *>(lua_tostring(L, -1));
+    a = reinterpret_cast<const char *>(lua_tostring(L, -1));
     if (a != NULL) {
         luaRet.assign(a);
     }
 
     t->debug(9, "Returning from lua script: " + luaRet);
 
-    lua_pop(L, 1);
-    lua_close(L);
-
     if (luaRet.size() == 0) {
-        return false;
+        ret = false;
     }
 
-    return true;
+
+err:
+    lua_pop(L, 1);
+    lua_close(L);
+
+    return ret;
 #else
     t->debug(9, "Lua support was not enabled.");
     return false;
@@ -287,6 +294,10 @@ int Lua::getvars(lua_State *L) {
         idx++;
     }
 
+    for (const collection::Variable * i : l) {
+        delete i;
+    }
+
     return 1;
 }
 
@@ -372,6 +383,7 @@ std::string Lua::applyTransformations(lua_State *L, Transaction *t,
                 t->debug(1, "SecRuleScript: Invalid transformation function: " \
                     + std::string(name));
             }
+            delete tfn;
         }
 
         return newVar;
@@ -388,6 +400,7 @@ std::string Lua::applyTransformations(lua_State *L, Transaction *t,
         // FIXME: transformation is not yet returning null.
         if (tfn) {
             newVar = tfn->evaluate(newVar, t);
+            delete tfn;
         } else {
             t->debug(1, "SecRuleScript: Invalid transformation function: " \
                 + std::string(name));

diff --git a/src/operators/fuzzy_hash.cc b/src/operators/fuzzy_hash.cc
@@ -91,7 +91,7 @@ FuzzyHash::~FuzzyHash() {
         free(c->data);
         c->data = NULL;
         c = c->next;
-        free (t);
+        free(t);
     }
     m_head = NULL;
 }

diff --git a/test/valgrind_suppressions.txt b/test/valgrind_suppressions.txt
@@ -3771,4 +3771,55 @@
    fun:_Z17perform_unit_testPN16modsecurity_test15ModSecurityTestINS_14RegressionTestEEEPSt6vectorIPS1_SaIS5_EEPNS_22ModSecurityTestResultsINS_20RegressionTestResultEEEPi
    fun:main
 }
+{
+   <insert_a_suppression_name_here>
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:realloc
+   fun:write
+   fun:_ZN11modsecurity6engine3Lua11blob_keeperEP9lua_StatePKvmPv
+   obj:/usr/lib/liblua.so.5.3.4
+   obj:/usr/lib/liblua.so.5.3.4
+   obj:/usr/lib/liblua.so.5.3.4
+   obj:/usr/lib/liblua.so.5.3.4
+   fun:_ZN11modsecurity6engine3Lua4loadENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPS7_
+   fun:_ZN11modsecurity10RuleScript4initEPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
+   fun:_ZN2yy14seclang_parser5parseEv
+   fun:_ZN11modsecurity6Parser6Driver5parseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES9_
+   fun:_ZN11modsecurity5Rules4loadEPKcRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
+   fun:_Z17perform_unit_testPN16modsecurity_test15ModSecurityTestINS_14RegressionTestEEEPSt6vectorIPS1_SaIS5_EEPNS_22ModSecurityTestResultsINS_20RegressionTestResultEEEPi
+   fun:main
+}
+{
+   <insert_a_suppression_name_here>
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:_Znwm
+   fun:_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE9_M_assignERKS4_
+   fun:assign
+   fun:operator=
+   fun:_ZN11modsecurity6engine3Lua4loadENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPS7_
+   fun:_ZN11modsecurity10RuleScript4initEPNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
+   fun:_ZN2yy14seclang_parser5parseEv
+   fun:_ZN11modsecurity6Parser6Driver5parseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES9_
+   fun:_ZN11modsecurity5Rules4loadEPKcRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
+   fun:_Z17perform_unit_testPN16modsecurity_test15ModSecurityTestINS_14RegressionTestEEEPSt6vectorIPS1_SaIS5_EEPNS_22ModSecurityTestResultsINS_20RegressionTestResultEEEPi
+   fun:main
+}
+{
+   <insert_a_suppression_name_here>
+   Memcheck:Leak
+   match-leak-kinds: definite
+   fun:_Znwm
+   fun:_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE12_M_constructIPcEEvT_S7_St20forward_iterator_tag.isra.137
+   fun:_M_construct_aux<char*>
+   fun:_M_construct<char*>
+   fun:basic_string
+   fun:RuleScript
+   fun:_ZN2yy14seclang_parser5parseEv
+   fun:_ZN11modsecurity6Parser6Driver5parseERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES9_
+   fun:_ZN11modsecurity5Rules4loadEPKcRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE
+   fun:_Z17perform_unit_testPN16modsecurity_test15ModSecurityTestINS_14RegressionTestEEEPSt6vectorIPS1_SaIS5_EEPNS_22ModSecurityTestResultsINS_20RegressionTestResultEEEPi
+   fun:main
+}