‘Trojan Source’ Bug Threatens the Security of All Code

> It'd also likely be pretty frustrating to end users if we were to highlight every single unicode character that looks like the latin alphabet.

That actually strikes me as very desirable. (Especially in light of the old maxim that "programs must be written for people to read, and only incidentally for machines to execute".)

> It'd also likely be pretty frustrating to end users if we were to highlight every single unicode character that looks like the latin alphabet.

Have you tried something similar to what the browsers do where highlighting is only enabled when there are multiple scripts mixed within the same token? Source code seems like it would be harder since you have many tokens rather than just a single one as in a hostname, and I'd be curious how much legitimate usage mixes scripts for technical reasons because you have something like a language or framework convention that certain names start with a particular English-derived term.

> It's certainly a good lesson in not copy/pasting random snippets from the internet...

For someone with more gumption than me:

Future copy & paste will default have intermediate screenshot and OCR steps. Voila: charset scrubbing for free.

Why not? Already today misc UIs and renderings disallow text selection. Drives me nuts.

> It's certainly a good lesson in not copy/pasting random snippets from the internet and pasting them into a root shell, however

I gotta say that I always make sure that I understand each piece of code that I copy paste but I do copy paste and never thought of this type of attack. Maybe that's something I should pay attention to in the future.

>These characters have valid use-cases in right-to-left languages like Arabic, Japanese etc,

I've never seen it used for Japanese. I don't think there is a valid use case for Japanese.

This doesn't feel particularly new either? Isn't it pretty much a new variant of https://github.com/reinderien/mimic ?

Which, if one is suspicious of code, can be defeated in vim with: set encoding=latin1

  this was a royal pain in the arse to figure out if I had live examples in the specs, because vim also just rendered them "correctly"

That's because vim supports Farsi/Arabic natively from day one. Even if the OS does not support it, you can still write bidirectional and right-to-left text in vim. Never knew the reason, but thanks Bram Molenaar.

I was intrigued by your meta example and I took a look. It took me 3-4 minutes to find the warning, and I was looking for it!

I was expecting a big fat warning on the merge request itself, or maybe on the lines containing the dangerous chars.

In the end, it is a small ? character inserted were the unicode control chars are, and a mouseover tooltip warning about a potential issue.

The warning is good, but why so subtle? Sorry for the criticism. The feature is still a huge positive.

I was impatient to find the example you were talking about; as far as I can tell, this is the line with the example: https://gitlab.com/gitlab-org/gitlab/-/commit/3fb44197195b57...

And here's what it looks like in various conditions/viewers:

With the fix, this is how it looks in the browser in the Gitlab interface:

    if (accessLevel != "user�") {� // Check if admin ��

Without the fix, viewed raw (and thus viewed in a vulnerable way), it looks like this:

    if (accessLevel != "user") { // Check if admin

And in a hex viewer, it looks like this:

    000005b0: 2020 2020 2020 2069 6620 2861 6363 6573         if (acces
    000005c0: 734c 6576 656c 2021 3d20 2275 7365 72e2  sLevel != "user.
    000005d0: 80ae 20e2 81a6 2f2f 2043 6865 636b 2069  .. ...// Check i
    000005e0: 6620 6164 6d69 6ee2 81a9 20e2 81a6 2229  f admin... ...")
    000005f0: 207b 0a20 2020 2020 2020 2020 2020 2020   {.
    00000600: 2063 6f6e 736f 6c65 2e6c 6f67 2822 596f   console.log("Yo
    00000610: 7520 6172 6520 616e 2061 646d 696e 2e22  u are an admin."

I personally wish that in repos with the warning enabled, that the �s were displayed in lieu of the malicious characters instead of in addition to them. For example, I'd rather see this:

          var accessLevel = "user";
          if (accessLevel != "user� �// Check if admin� �") {
              console.log("You are an admin.");
          }

than this:

          var accessLevel = "user";
          if (accessLevel != "user�") {� // Check if admin�� 
              console.log("You are an admin.");
          }

First place winner of last year's underhanded Solidity contest used exactly this trick: https://blog.soliditylang.org/2020/12/03/solidity-underhande...

It's also worth noting that if you're caught playing games like this, there is really no way to explain your actions that would avoid serious consequences.

If however, you used the "bugdoor" method, you can plausibly deny any malicious intent and you will absolutely get away with it.

Oldest trick in the book:

    /* Legitimate comment.        <a lot of white space to go off screen> */ #define malicious code

Nobody will notice the horizontal scroll bar.

> Fortunately, this is quite easy to fix - just don't accept any patches with source code that have any non-ASCII outside small set of localization resources

There are plenty of projects out there written by people who aren't English speakers who depend on the Unicode capabilities of languages to write code that is actually readable to them. Turning that off is far from a solution.

You argument away your own fix. Proposed fix is like if rm was limited to files outside of /sys, plenty of projects depend on the standardized behavior.

APL developers would disagree.

> The rm tool doesn't know which files are important to you, and the compiler doesn't - and shouldn't - know what you consider to be "correct" code.

This is actually no longer true. Many rm implementations today prevent you from deleting a path including the root directory, unless you explicitly specify `--no-preserve-root`. Similarly, a lot of compilers tend to warn you or outright stop if they detect code that is very likely to be buggy - the rust compiler warning about these control characters is just the latest example.

Of course, in theory, each tool should do its job and the user should be the boundary to know whats right. In practice, though, these heuristics tend to catch bugs-to-be 95% of the time (at least in my experience) and are easily disabled otherwise, so they are good to have.

That’s a really impressively written error message.

Their advisory is well-written and explains the problem well. The example code they use:

  if access_level != "user" { // Check if admin

opens up a whole can of worms though. You don't need cunning invisible control codes to break that line, you could just replace any of the letters in 'user' with a different, but almost-identical looking unicode symbol and you'd still have an exploit. Even better, this would be a completely deniable attack ("oops, I must have accidentally pressed alt-R while typing that letter" excuse) - whereas explaining away why you checked in some magical RTL/LTR encodings and hacked up a comment is impossible. Plus, it would render well in far more apps, terminals, command line programs, etc etc

You're right - their headline is written for attention. It's an exploit of a feature.

What I'm interested to know is whether there is any code already out there in the wild with this exploit in it? An intelligence service could have exploited this years ago without anyone noticing until now.

Unicode is a pathway to all manner of hijinks, including as you say, homoglyph attacks. For instance, on some TLDs I can easily create two different domain names that render identically in the browser.

https://github.com/rust-lang/rust/issues/28979 plenty of discussion here on Unicode including homoglyph attacks. This is for Rust but has links to Go and Zig. The Unicode standard also has extensive discussion, for example https://unicode.org/reports/tr31/ and http://unicode.org/reports/tr39/ on identifiers and security.

In general a multilayer solution is needed: compilers, linters, Unicode standard, merge tools, editors, and so on.

It's a feature for prose text, so programs like Word should support it. It's a security bug in anything designed to be parsed or interpreted by software, so programs like Visual Studio Code should refuse to honor it.

What's needed is to impose on programming languages, outside of comments, checks similar to the checks made for domain names.

There is a draft standard for this.[1] It references RFC 5893 and some other documents. Some of the rules:

- All code points in a single label must be taken from the same script as determined by the Unicode Standard Annex #24: Script Names. Exceptions to this guideline are permissible for languages with established orthographies and conventions that require the commingled use of multiple scripts. (Like mixing kanji and romaji in Japanese.)

- The "Bidi rules" of RFC 5893, which define allowed right to left and left to right modes, must be enforced. These are complicated, because of such things as the Arabic and Hebrew convention of right to left text with left to right numeric digits in numbers. But they are well-defined.

- Only code points allowed by IDNA 2008 are allowed. This eliminates such things as the non-breaking zero width space, the expansion areas for future use, and such.

The domain name people have been banging on this problem since 2003, and by now, there's a rough consensus of what to disallow. So start putting checks for that in compilers. If you find violations of those rules, it's more likely to be a typo than something useful, anyway.

So that's a way out of this.

[1] https://www.icann.org/en/system/files/files/draft-idn-guidel...

> "IDEs and compilers should ignore character-direction codes when looking at source files."

No I think some people would disagree, arabic coders for example. People just need to be aware of this when using unicode in their product.

browsers have solved it for domain names. you could apply the same heuristics for not mixing e.g. cyrillic and non cyrillic in the same word/file

Most web sites' comment sections will allow these. I think even Facebook allows tomfoolery like this. f̷̧̨̡̜͚̬̰͕̩̻̳̜̮̫̰̓̏̑̐̇̍͘͝è̶̙̥͚̰͈͚̹̜̼͙͚͌à̴̛̙̣̖̮̰̥̩͚̣̺͋̂̓̓̍͛͜ͅr̶̛͎̬̭̯͙͉̬̞̤̲̞̼͉̣̃̒͗̄̎̋͆̓̊͐͝ͅ ̷̦̬̳̹̦̳̭͓͖͔̺̮̩̓̆̍̊͆͊͜ẗ̵̩͙̼́̂̿́̂̈́̈́̑ḩ̸̨̝͇̖̤̱̠̼̣͈̩͈̰̃̂͂͋̏̇͗͛̓̈́̍̑͑̕̚͜ë̶̥̜̘͙̦̋ ̸̛̟̀̄̌͗́̄ͅų̷̪͙̰̜̟̰̦͇̞̥̜͓͇͊̆͗̆̀͗̀̈́̌̉͆̏͆͜ṫ̸̨̜͈̲̺͖͓̄̍̄̓̏̓̓̎͜ͅf̴̨̼̜̯͇̜̯̹͚͛8̴̧̤͖̠̳̜̤̻̟̏̀̽̃m̷̯̤̳͔̘̣͙̗̰͙͔̦̰̝͑͐͌̓̎̀̇̓̆͂̎̐̽̚͠ą̶̢̻͂̒͛̑̅̈͛͑̈́̓̚͠͝n̴̦̺͖̆̑͗̾̈̀̉̿̑̐̈́͝ͅͅ

I’m sure you created a shitty day for one of us :( I’d like to say this was unusual, but it was pretty common.

Having readable unicode in string literals is nice.

> Bruce Schneier wrote it when Unicode came out btw: "Unicode is too complex to ever be secure".

It's astounding to me that there's room for such complexity in it. I thought it was just a lot of symbols. What other rules does Unicode have besides changing the order sometimes?

Give it a few years and unicode will probably be turing-complete. For reasons... likely not good ones though.

That was my first thought -- run all your source through an ASCII-only filter, the problem goes away.

Seems the bigger complaint isn't lack of fancy unicode in comments its non-english speakers with non latin alphabets wont be able to comment in their native language.

I'll leave it up to others to discuss how important this is or isn't.

Though I comment source in English, lots of people that I work with comment in other languages. כולל עברית, מימין לשמאל.‏

Might be nice to have an easy tool to scan files and whitelist characters from specific alphabets, because in most international teams I think you'll have a common language for comments, and so I think it's unlikely that you'll need say European and Indic and Chinese characters in one code base. Except the one pain point I can see - @author annotations in the source code, if you have an international team you might end up with a variety of scripts in that field, in my mind that's something that can be lived without, but I can imagine some people being sensitive about that.

Wouldn't simply stripping comments before doing any other processing solve the problem? I know there are plenty of programs that sprinkle code into comments, from Emacs to linters. Or is this obviously naive?

Seems to me that if you need to put code in the comments, you've got a bigger problem. I know people like tab hints and lint overrides, but maybe it is time to focus on separation of concerns at a higher level?

Even ignoring the fact that non-english speakers will want to (and do; I worked on a compiler with SJIS support 20 years ago) write comments in their native languages, it's unavoidable to have non-ascii characters in your string literals unless you want to regress user-interfaces to 1980.

GitHub has already updated their UI I see

In case there are people who (currently) don’t have access to such an editor, here is a screenshot: https://i.imgur.com/2Ue2Vvd.png

You mean, like this?

https://imgur.com/a/unKuOoK

Snark aside, most text based editors have some giveaway or another. Even the GUI ones show syntax highlighting quirks that show that something is wrong.

This is only really relevant in unicode-aware terminals, without syntax highlighting and when you don't get to scroll between characters. IOW, it's really quite hard to do.

> IOCCC entries will absolutely become more fun though.

IOCCC doesn't allow unescaped octets with high bit set [1], so even that's no go.

[1] https://www.ioccc.org/2020/rules.txt (rule 13)

I wouldn't be so sure about visibility since it seems most code editors and programming languages want to support more unicode, not less... One of my hobbies used to be annually running a regex search through the company's millions of lines of java to see how much of an increase there was in non-printable spaces (0x200b) in java method names or other symbols. Eclipse at least wouldn't show them by default, I don't remember IntelliJ's behavior, but most people wouldn't know they were there. I was aware of only one time when it impacted someone who typed in a whole identifier by sight but the reference included a 200b and they were stuck for a bit figuring out why things didn't work.

But I agree the trick (hard to call it an attack or even bug) is fun, in the same way as the earlier tricks of fake filename extensions. And terribly obvious, even with the limitations of default code viewers, and with no plausible deniability once caught, so it's pretty overblown for practical considerations. The intentionally introduced Linux kernel bugs from several months ago were far more significant a lesson for people to learn from, and they didn't rely on any unicode tricks but on much simpler tricks that were also somewhat plausibly deniable to chalk up to an oopsie.

If I understand correctly, what you're suggesting could be thought of as pre-rendering directionality into the memory layout. If we did that then it might compromise our ability to write an algorithm that iterates over a string of hebrew or arabic characters. Display is super complicated and people don't agree on how to do it. For example, consider the arabic text عودة أبو تايه‎. If I sneak a latin A between all those characters to prevent the display algorithm from rearranging and shaping them, then that same string looks like this: عAوAدAةAأAبAوAتAاAيAه. Those are the same characters and you can confirm that yourself using:

    for c in 'عودة أبو تايه‎':
      print(unicodedata.name(c))

On the other hand if you want to romanize that string as EWDTA AEBW TAIH then all you need is a for loop and a switch statement, because the memory order is always left to right. We can also rest assured that if someone invents a better display algorithm, we won't need to do any database migrations, since the encoding itself doesn't need to change.

If you have a document with mixed languages, you need to be able to edit each language in its natural direction after the fact. That requires storing directionality in the document.

And keep in mind that if you store RTL text backwards, as you propose, every algorithm now has to be able to process backwards text. Backwards spellcheck is a lot of extra work...

Because visual representation is separate from the underlying data structure. A string container doesn't have a specific direction, only a relative one. I.e. This character comes before the next and after the previous. Adding the bidi control code, the string indicates when the visual ordering changes in this relative direction system.

You could absolutely design a new string container that assumes left to right at all times and cannot be changed, but then it's on the programmer to ensure that strings are copied or concatenated in the right direction, at the right location, and substrings searching becomes a minor headache. How would you concatenate an RTL string to a forced LTR string representation? You would have to work out whether the end of the string it LTR or RTL. If LTR, append directly. If RTL find the character where the direction changes and insert the string in there - much more expensive. Better to just append the string, using bidi codes where required, and let the frontend process the string to make the appropriate direction changes. Yes, you may need to search the string for the bidi code to know which direction you're going at the end of the string, but that's just a simple reverse string search for a single control character, and not a complex variable multi-byte search of inferred character directions by codepoint values.

I think the issue is in the locations of which bidi codes are rendered. They provide an inherent untrustworthy-ness to the text area they're rendered in, and so should be treated as an exception in critical situations. I've seen the reversed exe file name trick used for years, and every time I ask myself why that's even a thing? If the OS used file headers and magic numbers to determine file types instead of the filename, it would be less of an issue.

For source code, I would question the rendering of RTL text in a source code editor as it's an obvious issue for code safety. Ideally, all source code would be kept to the same origin language - doesn't have to be english, just consistent. Any non-conforming text should ideally be loaded from a resource rather than inline within the source code, to avoid foreign character contamination and allow easier identification of these issues. Further, source code rendering should only render identified safe control codes, and treat unsafe ones as raw binary values to be shown as such - i.e. \r and \n are safe, \b is unsafe, and bidi codes would also be unsafe. You could even go so far as to include them in the syntax highlighting, but that results in a dependency on syntax highlighting to show the semantics of the source code rather than the text alone.

Flip left and right in your idea, and you can try it out without learning a new language. Remember to implement word wrap.

Yes! This would also do away with a whole class of conflicts related to whitespace/formatting.

The thing about text is that it is barebones. Everyone can agree what the structure of text is (a stream of bytes with some ascii like encoding).

For representing code as more than text, you will lose so much tools that can handle your code, it's a massive set back. Add to that how much effort it takes to get people onboarded on your new representation, and things look bleak for adoption.

Finally, programmers really like looking under the hood. And with plain text, you know exactly what your code looks like in bytes.

Also helps with naming. Only need a value once or twice? Don’t bother trying to name it, just link it into the tree where it’s needed.

Since most progamming languages are based on english, non-english text in string literals is almost always user-facing and should be put in resource files to make translation into additional languages easier.

Identifiers and comments are a serious problem though. Many application domains use terms that are tricky to translate into english. The translations could be misleading, inappropriate or not unique. Sometimes they are just plain wrong or there is no english word that fits. All of these could cause misconceptions, confusion and bugs, and make reading and working with the code and the running system harder.

Non-English characters are quite useful in comments where you're explaining Unicode processing stuff, and in regexes working with the characters, and when you're using maths notation (proper symbols in comments, Greek letters for variables, etc.), and when you're drawing boxes in a terminal. I'm sure there are many more too.

But what if this is slipped into your compiler? Your operating system's kernel? A top voted Stack Overflow answer? You can't (or it's infeasible to) check and control everything.

Yes, you're totally safe then. I've never heard of standard libraries having problems that affect security, certainly not the str* family of functions.

sigh...Why does it have to be "all or nothing"? These logical fallacies are pretty much a standard in these discussions.

Either have 100%, ironclad security, or "Who cares? YOLO! STDs be damned" abandon?

We do what we can to make sure what we write is as good as possible.

I lock my car door, when I get out. I know that it won't stop a determined thief, but it will avoid problems from the casual knucklehead.

It's this research that prompted GitHub to show warnings, they didn't appear as of yesterday.

https://blog.rust-lang.org/2021/11/01/cve-2021-42574.html has a nice clear example.

Full Stack Chris reviews some code that he thinks says:

    if access_level != "user" { // Check if admin

This may be an open source project. This may be an internal bad egg (a very common threat; insider jobs are actually one of the absolute top risks to a company). Or this code may be injected by an attacker who has gained access to the repo and is leaving backdoors that they hope to survive long after their access is blocked or leaving backdoors to make deployed production systems vulnerable. Etc.

And Chris won't notice that the computer will execute:

    if access_level != "user{U+202E} {U+2066}// Check if admin{U+2069} {U+2066}" {

This is not just an attack on compiled languages. Scripting languages are just as vulnerable.

I'd say that neovim is bugged here and gedit is the one working properly rendering unicode as it should be

I forced it globally, are there reasons that's bad to do?

    (setf (default-value 'bidi-display-reordering) nil)

The BIDI issue looks pretty bad in emacs-gtk: the sneaky text is unnoticeable in lots of modes, unless the cursor just happens to scroll over it.

Why did you put "fix" in quotes? Isn't that an actual fix for this?

Seriously. State run espionage is 100x more likely

Was that capability dropped in the transition from sgml to XML? If so, can someone here on HN provide some pointers to the old discussion?

Yes, Mastodon has recently been discussing this. https://github.com/mastodon/mastodon/issues/2777

If the screen reader understands Bidi (which it needs to in order to support some languages), maybe not.

Because the Greeks transferred ownership.

A pre–commit hook is just a script that returns 0 to allow the commit. If you want to block commits containing certain characters, just make a hook that runs grep -v with a suitable search expression.

It's a problem in any environment where people can input Unicode characters. Reviewers might use tools that are not able to see those things.

At the same time, one can't just put a blanket ban on Unicode. It exists for a reason. People want to use their native languages to name identifiers, or at least to write comments. Restricting ourselves to ASCII again and thus forcing English on everybody is not a solution.

The point of the vulnerability is that you can't necessarily see the strange Unicode characters.