Skip to content

Commit

Permalink
Fix warning reported by clang (possible bug in Settings lua api)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sfan5
sfan5 committed Dec 21, 2016
1 parent 04f68a8 commit cc7c31a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/script/lua_api/l_settings.cpp
View file
Expand Up @@ -194,7 +194,7 @@ void LuaSettings::Register(lua_State* L)
int LuaSettings::create_object(lua_State* L)
{
NO_MAP_LOCK_REQUIRED;
bool write_allowed;
bool write_allowed = true;
const char* filename = luaL_checkstring(L, 1);
CHECK_SECURE_PATH_POSSIBLE_WRITE(L, filename, &write_allowed);
LuaSettings* o = new LuaSettings(filename, write_allowed);
Expand Down

4 comments on commit cc7c31a

@ShadowNinja
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's it unfortunately, since checkPath unconditionally sets it to false (if it's passed) before doing anything just to be safe.

@sfan5
Copy link
Member Author

@sfan5 sfan5 commented on cc7c31a Dec 21, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't fix #4943 for the case where mod security is enabled, it is only supposed to fix the case where mod security is disabled, CHECK_SECURE_PATH_POSSIBLE_WRITE does nothing and write_allowed is used uninitialized.

Besides, I have actually verified that this fixes the bug with mod security disabled.

@est31
Copy link
Contributor

@est31 est31 commented on cc7c31a Dec 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ShadowNinja From looking at the definition of CHECK_SECURE_PATH_POSSIBLE_WRITE it should be obvious that this change is needed.

@ShadowNinja
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfan5: Oh, you're right. This must have been the cause of #4943, mod security was off.

Please sign in to comment.