Minetest logo

IRC log for #minetest-dev, 2020-02-06

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
00:12 AntumDeluge joined #minetest-dev
00:12 erlehmann joined #minetest-dev
00:15 xerox123_2 joined #minetest-dev
00:18 tuedel_ joined #minetest-dev
00:19 behalebabo_ joined #minetest-dev
00:19 Amaz_ joined #minetest-dev
00:23 ircSparky joined #minetest-dev
00:31 IcyDiamond joined #minetest-dev
00:31 rom1504 joined #minetest-dev
00:41 fluxflux joined #minetest-dev
00:52 LoneWolfHT joined #minetest-dev
00:54 Icedream joined #minetest-dev
01:05 Foz joined #minetest-dev
01:14 erlehmann joined #minetest-dev
01:41 Lone_Wolf joined #minetest-dev
05:33 ssieb joined #minetest-dev
06:08 rtalk101 joined #minetest-dev
06:19 Edgy1 joined #minetest-dev
08:04 fluxflux joined #minetest-dev
08:38 ShadowNinja joined #minetest-dev
09:49 erlehmann joined #minetest-dev
09:54 erlehmann joined #minetest-dev
10:00 erlehmann joined #minetest-dev
10:02 twoelk joined #minetest-dev
10:37 df458 joined #minetest-dev
11:20 proller joined #minetest-dev
11:37 Fixer joined #minetest-dev
11:53 Fixer_ joined #minetest-dev
12:03 proller joined #minetest-dev
12:14 calcul0n joined #minetest-dev
12:29 Beton joined #minetest-dev
13:06 fluxflux joined #minetest-dev
13:14 ANAND joined #minetest-dev
13:35 absurb joined #minetest-dev
13:56 Zeno` joined #minetest-dev
14:29 ANAND joined #minetest-dev
14:57 Lone_Wolf joined #minetest-dev
15:13 Lone_Wolf joined #minetest-dev
15:24 Wuzzy joined #minetest-dev
16:02 proller joined #minetest-dev
16:08 pmp-p joined #minetest-dev
16:28 proller joined #minetest-dev
17:26 nepugia joined #minetest-dev
17:32 Krock joined #minetest-dev
17:35 proller joined #minetest-dev
17:46 proller joined #minetest-dev
18:08 proller joined #minetest-dev
18:53 sfan5 merging game#2579 in 5m
18:53 ShadowBot https://github.com/minetest/minetest_game/issues/2579 -- Add traditional Chinese translation(default) by IFRFSX
19:10 Krock (7 mins)
19:13 erlehmann joined #minetest-dev
19:15 sfan5 oh well
19:20 Krock what does the mgv7 "ridges" mgflag do?
19:21 Krock ooh. the GUI translates that to "Rivers"
19:29 paramat joined #minetest-dev
19:30 paramat i might merge game#2586 in 3 hours
19:30 ShadowBot https://github.com/minetest/minetest_game/issues/2586 -- Avoid error when 'get_player_information' returns 'nil' by paramat
19:31 paramat game#2580 simple high priority bugfix
19:31 ShadowBot https://github.com/minetest/minetest_game/issues/2580 -- Fix missing papyrus in savanna, add a dry dirt version by paramat
19:47 ssieb joined #minetest-dev
20:31 proller joined #minetest-dev
20:38 Miner_48er joined #minetest-dev
20:40 Miner_48er joined #minetest-dev
20:46 Wuzzy Krock: ridges are those wide river channels that agressively cut through terrain, often generating huge cliffs as well
20:51 Krock Wuzzy: in the meantime I also figured that out. Thanks for confirming.
20:56 Kray joined #minetest-dev
21:33 df458 joined #minetest-dev
22:03 Fixer joined #minetest-dev
22:50 Wuzzy I am worried about the security of minetest.deserialize, based on what I learned from #9369
22:50 ShadowBot https://github.com/minetest/minetest/issues/9369 -- Fix misleading documentation on minetest.deserialize(). by luk3yx
22:51 Wuzzy (my security concern is independent from that PR. it's because the PR claims that this function may execute Lua code)
22:53 Wuzzy If I understand correctly, if any user input is thrown into minetest.deserialize, you have an exploit?
22:54 sfan5 before this PR: correct, after this PR: not with mod security enabled
22:55 Wuzzy ouch!
22:55 sfan5 in in either case not at all if you passed the undocumented (wtf?) safe=true parameter
22:55 sfan5 s/^in/and/
22:56 Wuzzy but "This function should not be used on untrusted data, regardless of the
22:56 Wuzzy value of safe."
22:57 sfan5 that mostly precautionary
22:57 sfan5 well
22:57 sfan5 denial of service / resource exhaustion is trivially possible even with safe=true
22:58 Wuzzy i'm more like thinking about putting stuff like lua functions into a server
22:58 Wuzzy to introduce cheats etc
22:59 Wuzzy Why does this function have to be unsafe in the first place? could this (de)serlialization not be done differently? without executing arbitrary code?
23:01 sfan5 depending on the attack vector, the exploiter will have to get creative to introduce cheats that last after a server restart (especially with mod security enabled)
23:02 sfan5 parsing serialized lua tables manually would be lots of extra work and complexity.
23:02 Wuzzy sure, but it still seems super silly to have an unsafe deserialize
23:03 Wuzzy hmmm, if all mod security is down, whats the worst possible damage?
23:03 Wuzzy is lua allowed to arbitrary write+read any random file it can find?
23:04 sfan5 yes?
23:04 sfan5 what do you think mod security is about
23:04 Wuzzy 😀
23:04 Wuzzy its been a long time since i last touched this thing. i kinda remember now...
23:05 Wuzzy Any idea for a safe implementation of deserialize?
23:06 sfan5 write a lexer, parser and interpreter for the extremely limited subset of lua minetest.serialize() outputs
23:06 sfan5 like I said, it's lots of extra work
23:07 Wuzzy rriiiiiiight. that's going to be !!!FUN!!!
23:07 sfan5 the other alternative is to try to validate the string before running it, but it's hard to be sure that that's safe
23:07 Wuzzy so i suppose loadstring is basically only used because its easy and it works
23:08 rubenwardy Using load string is fine when you sandbox it
23:08 rubenwardy Ie: setfenv
23:09 rubenwardy However, using Lua to serialise is troublesome to begin with, JSON would be preferable
23:09 sfan5 if you consider loadstring("while true do end")() to be "safe" yes
23:09 Wuzzy hahahahaha
23:10 Wuzzy looks like loadstring just dumps whatever it loads into the global env? boo!
23:10 sfan5 no
23:10 Wuzzy Lua 5.3 lets you at least pick environment with `env` param
23:10 sfan5 loadstring returns a function whose body is the code you gave
23:10 Wuzzy https://www.lua.org/manual/5.3/manual.html#pdf-load
23:10 sfan5 load != loadstring
23:11 rubenwardy You can use coroutines to limit execution time
23:11 rubenwardy But sandboxing is broken on LuaJIT
23:11 Wuzzy note that loadstring does not exist in 5.3. it has been renamed to load
23:11 p_gimeno rubenwardy: not if you disable the JIT
23:12 rubenwardy Sounds expensive
23:12 sfan5 oh cool
23:12 sfan5 Wuzzy: in any case you can pick an env in 5.[12] too by using setfenv, exactly what the PR does
23:13 rubenwardy Are there any cases where clients can provide data to deserialise?
23:13 Wuzzy ah thats what this setfenv does. nice
23:13 rubenwardy Presumably the server doesn't allow clients to change item meta
23:14 Wuzzy of course. deserialize is so frequently used, it would surprise me if *some* user input is serialized soewhere
23:14 Wuzzy its a game thing
23:14 Wuzzy but still
23:14 rubenwardy Having user input being serialised is fine
23:14 Wuzzy this is unexpected, and a big trap for game+mod makers
23:14 Wuzzy really?!
23:14 rubenwardy The problem is when user input is deserialised
23:14 Wuzzy lol
23:14 rubenwardy Like, if the user directly provides the string to deserialise
23:15 sfan5 "do not unserialize arbitrary adversarial input" is not exactly unexpected
23:15 rubenwardy Yeah
23:15 sfan5 the docs should have made it clear though
23:15 Wuzzy thats what i'm saying
23:15 rubenwardy I can't think of any case where user input would be directly deserialised
23:16 rubenwardy But yes, it should be made clear
23:16 Wuzzy well it was unexpected to me. I did not know that deserialize is so deep into lua and actually leads to code execution
23:16 rubenwardy I suggest accepting that it can be used to ddos but otherwise sandboxing like in that PR
23:16 Wuzzy I don't think games will directly let users input a string into deserialize
23:17 Wuzzy however, it is much more likely that they will dump partial data into it. like  a string or number
23:17 sfan5 what?
23:18 sfan5 again, core.deserialize(core.serialize(input)) will never lead to insecurities
23:18 Wuzzy how?
23:18 sfan5 what "how"?
23:19 rubenwardy Because any user input is a string, not a function
23:19 Wuzzy why will it never lead to insecurity?
23:19 Wuzzy i see
23:19 sfan5 even if it were a function that would not be a problem (also that doesn't work with mod security on)
23:19 rubenwardy The security issue is if you allow  core.deserialize(input)   not if you allow  core.deserialize(core.serialize(input))
23:20 Wuzzy I still like the suggestion of rubenwardy to switch to a real data format instead of weird lua magic
23:20 rubenwardy Core.parse/write JSON exists
23:20 sfan5 that has some limitations IIRC
23:20 rubenwardy Yeah, because it's JSON
23:20 rubenwardy Wait
23:20 rubenwardy Does it support mixed integer and string keys?
23:21 Wuzzy what do u mean?
23:21 p_gimeno I've been researching Lua serialization because of these security issues, and found a couple libraries that can deal with Lua data
23:21 sfan5 rubenwardy: json as a format does not support that
23:21 p_gimeno but for this case, only one I think
23:21 Wuzzy actually, the only  stuff that needs serialization is: tables, numbers, strings, userdata. anything else can be ignored imo
23:21 Wuzzy like functions. pointless to serialize those
23:21 sfan5 >userdata
23:21 sfan5 no
23:22 rubenwardy Well, I discourage using mixed integer and string keys
23:22 Wuzzy userdata like ItemStack
23:22 rubenwardy ItemStacks should be serialisable
23:22 p_gimeno https://github.com/gvx/Smallfolk
23:22 Wuzzy right
23:22 rubenwardy Lol
23:22 Wuzzy each userdata type should have its own serialization
23:22 sfan5 disagree on that
23:22 sfan5 modders should ensure whatever objects they want to serialize are first converted to strings
23:23 Wuzzy well ItemStack for startes, but they already have to_string()
23:24 Wuzzy if we only care about tables, numbers and strings, then JSON has everything for us
23:24 sfan5 not really
23:24 Wuzzy what''s missing?
23:25 Wuzzy ok. nil. thats null in json...
23:25 rubenwardy Keys can only be strings
23:25 Wuzzy oops
23:25 rubenwardy Can't have recursive structures
23:25 Wuzzy hmmmmm
23:25 sfan5 well, to_json helpfully converts tables that "look" like arrays to actual json arrays
23:26 rubenwardy Can't have structures with duplicate nodes
23:26 sfan5 but e.g. { [-1] = "foo" } or { [1] = "foo" } are impossible in json
23:26 Wuzzy i cant remember when i ever wanted to save a recursive structure
23:26 rubenwardy Ie: the same table appearing twice
23:26 sfan5 p_gimeno: looks interesting but I think we could invent our own in C++ for more performance
23:26 rubenwardy JSON only supports a tree
23:26 Wuzzy ah you mean stuff like references?
23:27 rubenwardy Tables are always references
23:27 Wuzzy yes
23:27 proller joined #minetest-dev
23:27 sfan5 Wuzzy: it's extremely rare to want to do that honestly, but it's still something json can't do over core.serialize
23:27 rubenwardy So I mean that the same table could appear twice in the parent table, JSON would duplicate that
23:27 Wuzzy i think you could do a workaround
23:28 Wuzzy add some kind of "reference table" in which each table appears. and a "data table" in which the real data is inserted. tables are then refered by their reference
23:29 Wuzzy not sure if that works, however. its a hack
23:29 sfan5 then it's not JSON anymore and we'd be better off inventing our own format
23:29 Wuzzy welll it would technically still be json, but with trickery lol
23:29 p_gimeno IMO deserialize() should be a custom parser that parses a subset of Lua, for compatibility
23:30 p_gimeno and serialize() should stay as is
23:30 rubenwardy I don't think it's worth it given that having direct user input is rare, and we can limit the issue to ddos rather than full execution
23:30 p_gimeno I can do that, I've written language parsers
23:31 Wuzzy hmmmm let me think
23:31 Wuzzy if the only thing an user can input are strings, could I even do DDoS?
23:31 Wuzzy ok maybe restrict length to 1000 characters first
23:31 rubenwardy If you can do direct input, then you can do a while loop
23:32 rubenwardy core.deserialize("while true do end")
23:32 Wuzzy ok a direct input into deserialize is very wrong, i never did that
23:32 Wuzzy thats not what i meant
23:32 rubenwardy It's safe when you never do a direct input
23:32 sfan5 perhaps validating the input string is not that hard
23:32 Wuzzy i meant a string that is part of a bigger data structure that is then (de)serializeD)
23:33 rubenwardy That's fine
23:33 sfan5 after ignoring string contents, there should be no parentheses (so no function calls possible) and no language keywords left
23:33 Wuzzy e.g.
23:33 Wuzzy t.some_string = get_user_input()
23:33 Wuzzy minetest.serialize(t)
23:33 rubenwardy That's fine and not effected by this issue
23:33 Wuzzy minetest.deserialize(previous_input_oops_i_forgot)
23:33 sfan5 except "return" and local" since serialize uses that for references
23:34 rubenwardy The only risk there is the size of the user input  which should probably be limited on the network or something
23:34 Wuzzy assuming that get_user_input only outputs string
23:34 Wuzzy I usually use (de)serialize for 2 things:
23:34 sfan5 Wuzzy: get_user_input can quite literally return anything, be it a table, string, int, float, function or _G
23:34 sfan5 <sfan5> again, core.deserialize(core.serialize(input)) will never lead to insecurities
23:35 Wuzzy 1) saving entity data un unload
23:35 Wuzzy 2) saving data to disk
23:35 Wuzzy however, the 2nd thing can be avoided when using mod data
23:36 Wuzzy mod storage*
23:36 Wuzzy entity data is the main use case for me, so you have a neat string for get_staticdata()
23:48 twoelk left #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext