AP_Scripting: fix CRSF leaks and nullptr dereferences

libraries/AP_RCTelemetry/AP_CRSF_Telem.cpp

@@ -2000,6 +2000,10 @@ AP_CRSF_Telem::ScriptedMenu* AP_CRSF_Telem::ScriptedMenu::add_menu(const char* m
     }
 
     ScriptedMenu* menu = NEW_NOTHROW ScriptedMenu(menu_name, size, parent_menu);
+    if (menu == nullptr) {
+        return nullptr;
+    }
+
     menu->id = tail->id == 0 ? SCRIPTED_MENU_START_ID : tail->id + MAX_SCRIPTED_MENU_SIZE + 1;
     tail->next_menu = menu;
 

libraries/AP_Scripting/AP_Scripting_CRSFMenu.cpp

@@ -10,17 +10,11 @@
 
 AP_CRSF_Telem::ScriptedParameter* CRSFMenu::add_parameter(uint8_t length, const char* data)
 {
-    if (menu == nullptr) {
-        return nullptr;
-    }
     return menu->add_parameter(length, data);
 }
 
 AP_CRSF_Telem::ScriptedMenu* CRSFMenu::add_menu(const char* menu_name)
 {
-    if (menu == nullptr) {
-        return nullptr;
-    }
     return menu->add_menu(menu_name, 2, menu->id);
 }
 
@@ -30,13 +24,16 @@ int lua_CRSF_new_menu(lua_State *L)
 
     const char * name = luaL_checkstring(L, 1);
 
+    // create Lua object first in case of errors so we don't leak the menu
+    auto *m = new_CRSFMenu(L);
+
     AP_CRSF_Telem::ScriptedMenu* menu = AP::crsf_telem()->add_menu(name);
 
     if (menu == nullptr) {
-        return luaL_error(L, "No menu named: %s", name);
+        return luaL_error(L, "out of memory");
     }
 
-    *new_CRSFMenu(L) = CRSFMenu(menu);
+    *m = CRSFMenu(menu);
 
     return 1;
 }
@@ -104,14 +101,14 @@ int lua_CRSF_add_parameter(lua_State *L)
     CRSFMenu* ud = check_CRSFMenu(L, 1);
     size_t len = 0;
     const char * data = luaL_checklstring(L, 2, &len);
+
+    // create Lua object first in case of errors so we don't leak the param
+    auto *p = new_CRSFParameter(L);
+
     AP_CRSF_Telem::ScriptedParameter* param = ud->add_parameter(len, data);
 
     if (param) {
-#if 2 > LUA_MINSTACK
-        luaL_checkstack(L, 2, nullptr);
-#endif
-
-        *new_CRSFParameter(L) = CRSFParameter(ud->menu, param);
+        *p = CRSFParameter(ud->get_menu(), param);
         return 1;
     }
     return 0;
@@ -123,13 +120,14 @@ int lua_CRSF_add_menu(lua_State *L)
 
     CRSFMenu* ud = check_CRSFMenu(L, 1);
     const char * name = luaL_checkstring(L, 2);
+
+    // create Lua object first in case of errors so we don't leak the menu
+    auto *m = new_CRSFMenu(L);
+
     AP_CRSF_Telem::ScriptedMenu* menu = ud->add_menu(name);
 
     if (menu) {
-#if 2 > LUA_MINSTACK
-        luaL_checkstack(L, 2, nullptr);
-#endif
-        *new_CRSFMenu(L) = CRSFMenu(menu);
+        *m = CRSFMenu(menu);
         return 1;
     }
     return 0;
@@ -140,14 +138,14 @@ int lua_CRSF_add_root_menu(lua_State *L)
     binding_argcheck(L, 2);
 
     const char * name = luaL_checkstring(L, 2);
+
+    // create Lua object first in case of errors so we don't leak the menu
+    auto *m = new_CRSFMenu(L);
+
     AP_CRSF_Telem::ScriptedMenu* menu = AP::crsf_telem()->add_menu(name);
 
     if (menu) {
-#if 2 > LUA_MINSTACK
-        luaL_checkstack(L, 2, nullptr);
-#endif
-
-        *new_CRSFMenu(L) = CRSFMenu(menu);
+        *m = CRSFMenu(menu);
         return 1;
     }
     return 0;
@@ -159,6 +157,10 @@ int lua_CRSF_param_data(lua_State *L)
 
     CRSFParameter * ud = check_CRSFParameter(L, 1);
     AP_CRSF_Telem::ScriptedParameter* param = ud->get_parameter();
+    if (param == nullptr) {
+        return luaL_error(L, "internal error: %s is null", "CRSFParameter");
+    }
+
     lua_pushlstring(L, param->data, param->length);
     return 1;
 }

libraries/AP_Scripting/AP_Scripting_CRSFMenu.h

@@ -38,9 +38,12 @@ class CRSFMenu {
     uint8_t id() const { return menu->id; }
     const char* name() const { return menu->name; }
     uint8_t num_params() const { return menu->num_params; }
+    AP_CRSF_Telem::ScriptedMenu* get_menu() const { return menu; }
     AP_CRSF_Telem::ScriptedParameter* add_parameter(uint8_t length, const char* data);
     AP_CRSF_Telem::ScriptedMenu* add_menu(const char* menu_name);
 
+
+private:
     AP_CRSF_Telem::ScriptedMenu* menu;
 };