missing formspec escapes #5925

New Issue

tour · 2024-01-06T22:31:37Z

tour commented

2024-01-06 22:31:37 +00:00

seeing #5921 & #5920 I remember that I once found a missing formspec escape.
I showed Alias, but forgot to report it here.

I later did a grep for bad formspec patterns (which produced lots of false positives) and can add some more:

locks
mobs_core and mobs_redo nametag
moremesecons craftable command block
petz christmas present

seeing #5921 & #5920 I remember that I once found a missing formspec escape. I showed Alias, but forgot to report it here. I later did a grep for bad formspec patterns (which produced lots of false positives) and can add some more: - locks - mobs_core and mobs_redo nametag - moremesecons craftable command block - petz christmas present

👍 5

AliasAlreadyTaken added the

1. kind/bug

3. source/mod upstream

labels 2024-01-06 22:49:53 +00:00

AliasAlreadyTaken commented

2024-01-06 22:50:15 +00:00

Mind to share your grep, so we (and other servers) can reproduce it on their code?

tour commented

2024-01-06 22:54:47 +00:00

Forgot the exact command. Was on my old laptop.
I might find it in the bash history there, but I won't have access to that machine before tomorrow night.

Forgot the exact command. Was on my old laptop. I might find it in the bash history there, but I won't have access to that machine before tomorrow night.

👍 1

flux added the

2. prio/critical

label 2024-01-07 02:27:45 +00:00

flux commented

2024-01-07 02:28:50 +00:00

i don't think this sort of bug is easily grep-able, though it could be detected with good integration testing. "wishes for 2024"

👍 1

tour commented

2024-01-07 21:10:14 +00:00

i don't think this sort of bug is easily grep-able

very true, around >90% of my grep are false-positive...

first attemp: grep -r -n ";\"[\n ]*\.\." (eg. grep for ;" .. since that's what most people use to create formspecs)
Note that this won't catch formspecs build with string.format (I sometimes use this methods, but I haven't seen it somewhere else).
doesn't work if someone starts their concatenation without a ; in the end of their formspec strings.

But this pattern creates way too much false-positives.
From looking into the result, I noticed that many modders formspec-escape while building the fs-string, so I changed the pattern to
grep -r -n ";\"[\n ]*\.\.[^(]*]"
Note that there are still tons of false positives (As mentioned in my first comment)
this pattern might also let some bad boys through, but will therefore remove many false positives.

Soo...
I don't really see a good way to find missing formspec escapes.
some patterns might help to find places where formspecs are build.
bad boys might still not be found by this pattern, but I've never seen such a bad boy (eg. I'm quite sure they don't exist / The cost/benefit ratio is not worth looking for them)

The grep output to find the examples in my first comment was around 100-200 lines. (But I used the first pattern, which included way more false positves (learned a lot about patterns since these days)
If the grep output from YL is <1k <500 lines, I would volunteer to check them out and document my results here.

> i don't think this sort of bug is easily grep-able very true, around >90% of my grep are false-positive... first attemp: `grep -r -n ";\"[\n ]*\.\."` (eg. grep for `;" ..` since that's what most people use to create formspecs) Note that this won't catch formspecs build with `string.format` (I sometimes use this methods, but I haven't seen it somewhere else). doesn't work if someone starts their concatenation without a ; in the end of their formspec strings. But this pattern creates **way too much** false-positives. From looking into the result, I noticed that many modders formspec-escape while building the fs-string, so I changed the pattern to `grep -r -n ";\"[\n ]*\.\.[^(]*]"` Note that there are still tons of false positives (As mentioned in my first comment) this pattern might also let some bad boys through, but will therefore remove many false positives. Soo... I don't really see a good way to find missing formspec escapes. some patterns might help to find places where formspecs are build. bad boys might still not be found by this pattern, but I've never seen such a bad boy (eg. I'm quite sure they don't exist / The cost/benefit ratio is not worth looking for them) The grep output to find the examples in my first comment was around 100-200 lines. (But I used the first pattern, which included way more false positves (learned a lot about patterns since these days) If the grep output from YL is <~~1k~~ <500 lines, I would volunteer to check them out and document my results here.

tour referenced this issue

2024-01-09 11:09:05 +00:00

rheo reports: digistuff:touchscreen can be u ... #5923

tour commented

2024-01-21 01:39:08 +00:00

started to fix some of the known problems:

petz (already merged while writing this): https://github.com/runsy/petz/pull/207
moremesecons (craftable command block): https://github.com/minetest-mods/MoreMesecons/pull/34
mobs redo: https://codeberg.org/tenplus1/mobs_redo/pulls/9
not sure about mobs_core (according to /mods it's on YL): https://content.minetest.net/packages/mt-mods/mob_core/ says it is no longer supported. Their mobs_core:nametag is also affected. But I guess they are not obtainable on YL? (at least I can't find them in the crafting grid...)

started to fix some of the known problems: - petz (already merged while writing this): https://github.com/runsy/petz/pull/207 - moremesecons (craftable command block): https://github.com/minetest-mods/MoreMesecons/pull/34 - mobs redo: https://codeberg.org/tenplus1/mobs_redo/pulls/9 not sure about mobs_core (according to /mods it's on YL): https://content.minetest.net/packages/mt-mods/mob_core/ says it is no longer supported. Their mobs_core:nametag is also affected. But I guess they are not obtainable on YL? (at least I can't find them in the crafting grid...)

AliasAlreadyTaken commented

2024-01-21 01:44:42 +00:00

We're throwing out mob_core, don't bother

flux added the

4. step/ready to QA test

4. step/partially fixed

labels 2024-01-21 23:14:02 +00:00

flux commented

2024-01-21 23:14:53 +00:00

petz upstream is merged

👍 1

tour commented