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

rubenwardy · nerzhul · commit 1e3e4fb64928 · 2019-03-12T08:56:56.000+01:00
* HPChange Reason: Fix push after free, and type being overwritten Fixes #8227 and #8344
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;
 }

Original file line number	Diff line number	Diff line change
`@@ -257,6 +257,9 @@ int ObjectRef::l_set_hp(lua_State *L)`
`257`	`257`	`if (co->getType() == ACTIVEOBJECT_TYPE_PLAYER)`
`258`	`258`	`getServer(L)->SendPlayerHPOrDie((PlayerSAO *)co, reason);`
`259`	`259`
	`260`	`+ if (reason.hasLuaReference())`
	`261`	`+ luaL_unref(L, LUA_REGISTRYINDEX, reason.lua_reference);`
	`262`	`+`
`260`	`263`	`// Return`
`261`	`264`	`return 0;`
`262`	`265`	`}`