Implement mitigations for CVE-2021-42574 (Unicode right-to-left override attack)

[mrakh]

Sorry, I wasn't sure whether this should be filed under "bug" or "proposal", so I just chose the latter. Here is some context about the CVE, which is inherent to the Unicode spec.

In summary, it seems that the RIGHT-TO-LEFT OVERRIDE (U+202E) can be abused against font renderers that recognize it, to display source code in a manner different from its underlying textual content. Here is a Zig example (translated from the Rust example in the above link) of how this could be abused:

if (!std.mem.eql(u8, access_level, "user{U+202E} {U+2066}// Check if admin{U+2069} {U+2066}"))  {

Will be rendered by bidirectional-aware editors as:

if (!std.mem.eql(u8, access_level, "user"))  { // Check if admin

The Rust compiler has a mitigation for this as of v1.56.1, which throws an error if right-to-left-related control codepoints are found in unescaped form in source code. Perhaps the Zig compiler could do the same?

[jedisct1]

Yep, we should absolutely mitigate this as well.

[komuw]

Alternative takes on the issue:

• https://research.swtch.com/trojan - Russ Cox
• https://twitter.com/FiloSottile/status/1455260886910783501
• https://twitter.com/BRIAN_____/status/1455216970047975424

Both make the argument that this is not an issue to be fixed by compilers.

[jedisct1]

These are all very good points :)

[andrewrk]

Personally I am not convinced of a threat model that includes the attacker's ability to edit source code. A comprehensive response to such a threat model would look very different than haphazardly mitigating various flashy instances of the problem.

The zig compiler sees string literals as a series of bytes and is not aware of any visual properties. If you were going to point this CVE to the real culprit it would be text editors and/or Unicode.

The mitigations here are effectively augmenting text editors, adding a missing feature to the compiler that belongs in the text editor.

[SpexGuy]

Rust's solution here is bad, but this is not necessarily something that can be fixed in a text editor. The reverse characters are necessary for languages like arabic, so just disallowing it is not a good solution and will probably be reflected on poorly within the international rust community. The correct fix is to require that all unicode state has been reset at the end of a string literal or comment. Doing that requires an understanding of what constitutes a string literal or comment. It is unreasonable to require that Zig is only used with editors that have an understanding of Zig's syntax.

[SpexGuy]

Nevermind I'm an idiot, that doesn't solve the problem. The best we can do is have text editors fix this.

[jhg]

@andrewrk, it's not attacker's ability to edit our own code but attack the supply chain. And it's not limited to text in strings, where I agree with you (Zig sees it as bytes), but all code (examples in the paper and VSCode link). But VSCode show and GitHub warn about it, then it's mitigated currently.

Also, apart from attacks, the paper tell about compilers (and others) ignore formatting control characters, as if suggesting that must process that prior to parsing source code (III C). I'm not saying compilers should do that, Idk.

@SpexGuy, it only requires to be scaped and e.g. arab in comments can be used (I tried it with 1.56.1).

[SpexGuy]

The zig compiler rejects all non-ascii characters outside of string literals and escaped identifier literals (@"foo"), so for Zig it is only a problem in those cases.

[jhg]

@SpexGuy true, thx, I forgot that. Then I think in the end zig compiler was safe against that before of that paper and CVE.