| 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 |
