Time Nick Message 10:40 sfan5 merging #16530, #16506 soon 10:40 ShadowBot https://github.com/luanti-org/luanti/issues/16530 -- Fix a memory leak: Use irr_ptr for bones by appgurueu 10:40 ShadowBot https://github.com/luanti-org/luanti/issues/16506 -- Fix debug.getinfo not being unset in CPCSM by Desour 18:41 sfan5 any more thoughts on https://github.com/luanti-org/luanti/pull/16506#issuecomment-3323070923? 18:53 Krock there's definitely some edge-case where this could somehow be abused 18:53 Krock so blacklisting it in builtin for the server would make sense 19:01 jonadab Generally speaking, debug features should be off by default, and (usually) they should only be possible to turn on, for persons with an appropriate level of privilege/responsibility, e.g., the server admin in a server environment. 19:02 jonadab But that's very _general_ and I don't know the implications in this specific case. 19:02 Krock well, it depends. if you're using pcall/xpcall, then some debug tools might still be helpful? 19:03 jonadab It does somewhat depend, yes. And I should clarify that error messages sent even to abject end users, should be specific or unique enough to allow someone who gets a bug report with the error message in it, to identify the particular source of the message (line of code or whatever). 19:04 jonadab (Not that most users _actually_ give you the actual text of the error message most of the time, but we can dream.) 19:05 jonadab But there's a difference between a good error message, and a feature that lets an end user poke around in the program's internals or whatever. 19:08 MTDiscord well, many mods use debug.getinfo so its a bad idea to remove it 19:08 jonadab Ah. 19:08 Krock many mods? Do you have a list? 19:08 jonadab My question would be, what do they use it _for_ ? 19:09 jonadab (If mods have to resort to using a debug feature, to get information that they legitimately need, that sounds like an API design flaw.) 19:10 jonadab (Or at least an oversight.) 19:10 MTDiscord Krock: i don't have a list, but searching locally: fakelib, areas, arena_lib, futil, dbg (but that's expected), worldedit and some of my own mods 19:11 MTDiscord arena lib has the most intresting use assert(debug.getinfo(func).nparams <= 4, "[ARENA_LIB] Error! Since arena_lib 7.0, on_end callback can take only up to 3 parameters. " .. "Check the docs and fix the mod at fault accordingly") 19:12 jonadab areas and worldedit seem important to not break, but they should be actively maintained (one hopes), so if a change is needed, it may be possible to use a reasonable deprecation cycle or something. 19:12 MTDiscord areas uses it in its areas:registerProtectionCondition, but i dunno what its trying to do there 19:13 jonadab And yeah, a debug mod is obviously a special case here. 19:13 MTDiscord and also, Why get rid of debug.getinfo? 19:13 jonadab "get rid" is an overstatement; as I understand it, we're talking about making sure it's disabled by default, unless someone turns it on? 19:14 MTDiscord why? 19:14 Krock areas uses it for API misuse information. It would be possible to simply not provide the parent function reference 19:16 Krock worst case is some code duplication in C++ if we were to use a modified version of what's already contained in Lua (to implement debug.getinfo) 19:17 jonadab Another random thought: if some of the info it provides is safe and/or necessary, and other info that it provides is a security risk, could we be more granular about what info it will disgorge by default? 19:28 [MatrxMT] ...a breaking change, great. Of course I'm opposed, since arena_lib is a mod of mine 19:51 sfan5 nobody is suggesting to remove getinfo 19:52 sfan5 hopefully 19:55 rubenwardy https://content.luanti.org/zipgrep/9e84d971-357e-46ea-89d1-0cf858899fb2/ 19:55 rubenwardy ignore the exit 2, means no lua files found 19:57 sfan5 cellestial_game/mods/modlib/debug.lua: local func = debug.getinfo(stacklevel).func 19:57 sfan5 exile-rails/init.lua: local info = debug.getinfo(level, "f") 19:58 sfan5 ^ definitely sus 19:58 sfan5 dbg/src/dbg.lua: func = debug.getinfo((func or 1) + 1, "f").func <- this too 19:59 sfan5 the rest is probably fine 19:59 MTDiscord why would it be sus 20:03 MTDiscord i looked into exile-rails, seems to be written in typescript, so it may be a byproduct of whatever transpiler they were using 20:03 Krock impostors can be sus 20:03 sfan5 "sus" because they're retrieving the very property that would disappear as a security measure 20:04 Krock we could point it to any no-op function 20:04 MTDiscord oh 20:06 MTDiscord i think maybe as an extra security make debug.getinfo not be able to get functions of trusted mods, though i dont know if that would actually be a problem (the only issue with getinfo i see is that someone could use it to get access to a local function they arent supposed to, maybe?) 20:07 MTDiscord but that would require also being deeply aware of another trusted mod 20:07 MTDiscord so not that big of a deal 20:09 MTDiscord Krock: probably not a good idea, as that will silently break mods instead of giving you a clear error 20:09 MTDiscord i think disallowing getting functions of trusted mods is the best solution 20:10 sfan5 that's not the best solution at all, it's unreliable bandaid 20:11 sfan5 also is builtin considered a "trusted mod"? why or why not? 20:11 sfan5 (hint: answer is yes) 20:14 MTDiscord well, it wouldn't break existing mods/approaches (like using a strange transpiler) and it would most likely make abusing debug.getinfo impossible, in my view it is a good solution 20:15 MTDiscord and sure maybe debug.getinfo(some_builtin_func, "f") could be disallowed too /shrug 20:16 MTDiscord (edit: wait no im stupid, i meant debug.getinfo returning a builtin function) 20:31 luatic heh i need to update cellestiall game somewhen, debug.lua is long gone from modlib 20:31 luatic cellestial* 20:32 luatic sfan5: dbg is a debug mod, it is intended to look at upvals of functions among other things, it expects to be trusted to do this 20:43 sfan5 #16533 20:43 ShadowBot https://github.com/luanti-org/luanti/issues/16533 -- Restrict function references returned by debug.getinfo() by sfan5