[GDScript] Check string literals for Unicode direction control characters.

[bruvzg]

Fixes potential vulnerability, described in Trojan Source: Invisible Vulnerabilities. Since GDScript do not have {...} blocks, the second vulnerability variant probably is irrelevant.

Before:

After:

Test script:
dircontrol.gd.zip

Bugsquad edit: Mitigates CVE-2021-42574 for GDScript.

[mhilbrunner]

Good idea. Wonder whether it should be a warning instead of an error.

[bruvzg]

For the reference, here's an example of legitimate usage of direction control character: https://hosted.weblate.org/translate/godot-engine/godot/ar/?checksum=3818bad5b65a4bef#comments (using "\uXXXX" escape codes is more convenient way than adding raw characters, so this change won't break anything).

[akien-mga]

Good idea. Wonder whether it should be a warning instead of an error.

Rust seems to have added "deny by default" lints according to https://blog.rust-lang.org/2021/11/01/cve-2021-42574.html
Which would mean for us warnings treated as errors that could be disabled - but I don't think we have support to do that. Either all warnings are treated as errors, or none, IINM.

How would a typical non-exploit string using such Unicode direction control characters look like, both with and without escape codes? E.g. some Arabic with English words I suppose?

[bruvzg]

How would a typical non-exploit string using such Unicode direction control characters look like, both with and without escape codes? E.g. some Arabic with English words I suppose?

Here's a string for Weblate I mentioned before, but with escaped chars, control chars are used to keep English node names and punctuation in the correct order when they are inside the Arabic string.

معاملات غير صالحة للمشغل \u200f\u2068%s\u2069، \u2068%s\u2069\u200f و \u2068%s\u2069.

---

Invalid operands to operator Alpha, Beta and Gamma.

---

معاملات غير صالحة للمشغل Alpha، Beta و Gamma.
معاملات غير صالحة للمشغل ‏⁨Alpha⁩، ⁨Beta⁩‏ و ⁨Gamma⁩.

---

The first line is without control chars, the second with.

[akien-mga]

Thanks!

[Calinou]

but I don't think we have support to do that. Either all warnings are treated as errors, or none, IINM.

We should likely change each warning property from a boolean to an enum (Warning, Error, Ignore), and remove the warnings to errors setting in 4.0. This way, each warning can be configured to emit an error (and we can do this by default for a few warnings).

Edit: Proposal opened: godotengine/godot-proposals#3531