HPChange Reason: Fix push after free, and type being overwritten (#8359)

* HPChange Reason: Fix push after free, and type being overwritten Fixes #8227 and #8344
minetest · Mar 12, 2019 · 1e3e4fb · 1e3e4fb
1 parent 3b25b80
commit 1e3e4fb
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 5 deletions.
diff --git a/src/content_sao.h b/src/content_sao.h
@@ -405,6 +405,11 @@ struct PlayerHPChangeReason {
 	bool from_mod = false;
 	int lua_reference = -1;
 
+	inline bool hasLuaReference() const
+	{
+		return lua_reference >= 0;
+	}
+
 	bool setTypeFromString(const std::string &typestr)
 	{
 		if (typestr == "set_hp")

diff --git a/src/script/cpp_api/s_base.cpp b/src/script/cpp_api/s_base.cpp
@@ -384,14 +384,18 @@ void ScriptApiBase::objectrefGetOrCreate(lua_State *L,
 
 void ScriptApiBase::pushPlayerHPChangeReason(lua_State *L, const PlayerHPChangeReason &reason)
 {
-	if (reason.lua_reference >= 0) {
+	if (reason.hasLuaReference())
 		lua_rawgeti(L, LUA_REGISTRYINDEX, reason.lua_reference);
-		luaL_unref(L, LUA_REGISTRYINDEX, reason.lua_reference);
-	} else
+	else
 		lua_newtable(L);
 
-	lua_pushstring(L, reason.getTypeAsString().c_str());
-	lua_setfield(L, -2, "type");
+	lua_getfield(L, -1, "type");
+	bool has_type = (bool)lua_isstring(L, -1);
+	lua_pop(L, 1);
+	if (!has_type) {
+		lua_pushstring(L, reason.getTypeAsString().c_str());
+		lua_setfield(L, -2, "type");
+	}
 
 	lua_pushstring(L, reason.from_mod ? "mod" : "engine");
 	lua_setfield(L, -2, "from");

diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp
@@ -257,6 +257,9 @@ int ObjectRef::l_set_hp(lua_State *L)
 	if (co->getType() == ACTIVEOBJECT_TYPE_PLAYER)
 		getServer(L)->SendPlayerHPOrDie((PlayerSAO *)co, reason);
 
+	if (reason.hasLuaReference())
+		luaL_unref(L, LUA_REGISTRYINDEX, reason.lua_reference);
+
 	// Return
 	return 0;
 }