Comment by patrakov

3 years ago

Even more evil would have been to replace this line

    (void)SYS_landlock_create_ruleset;

with this:

    (void)SYS_landloсk_create_ruleset;

42 comments

patrakov

Reply
Terr_  3 years ago

For those squinting, the "landlock" regular "c" is replaced with a Cyrillic U+0441.

  • ak39  3 years ago

    Yip, this was impressive. Only a copy & paste into VSCode (Windows) revealed the rectangle around the "c" in the second line.

    Impressive.

  • SV_BubbleTime  3 years ago

    Is there a GCC option to error on non-standard English characters?

    • Terr_  3 years ago

      Disclosure: I've got zero C/C++ on my resume. I was asked to diagnose a kernel panic and backport kernel security patches once, but it was very uncomfortable. ("Hey, Terr knows the build system, that's close enough, right?")

      That said, perhaps something like disabling the default -fextended-identifiers [0], and enabling the -Wbidi-chars [1] warning.

      [0] https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html...

      [1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#inde...

      1 reply →

    • rurban  3 years ago

      Not yet. I was working on the standardization for C23, but this was postponed to C26, and has not much support. MSVC and SDCC liked it, CLANG and GC not so much.

      Here is my bad variant of the feature, via confusables: https://github.com/rurban/gcc/tree/homoglyph-pr103027

      The better variant would be to use my libu8ident, following UTR 39. I only did that for binutils.

    • masklinn  3 years ago

      Error-ing is the point here and what the period achieved. It’s a feature detection snippet so if it fails to compile the feature is disabled.

      8 replies →

    • patrakov  3 years ago

      Well - that would be exactly the point of the attacker. GCC errors out, and it does not matter whether this is because the intentionally typoed header does not exist or because non-English characters are not allowed. Errors encountered during compilation of test programs go to /dev/null anyway, as they are expected not to compile successfully on some systems - that's exactly the point of the test. So no, this option would not have helped.

    • pajko  3 years ago

      Probably the code review tools should be hardened as well, to indicate if extended identifiers had been introduced to a line where there wasn't any. That would help catching the replacement of a 'c' character with a Russian one.

      Btw, the -fno-extended-identifiers compiler parameter gives an error if UTF-8 identifiers are used in the code: <source>:3:11: error: stray '\317' in program float <U+03C9><U+2083> = 0.5f;

      2 replies →

    • fweimer  3 years ago

      The -fno-extended-identifiers option seems to do something in this area, but I don't know if it is sufficient. But it may block some characters which are used in standard English (for some values of standard).

      1 reply →

    • jcelerier  3 years ago

      I mean, GCC erroring is how the exploit works here. cmake tries to compile the source code: if it works then the feature is available, if it fails then the feature is not available. Forcing the build to fail is what the attacker wants.

saagarjha  3 years ago

Putting random Unicode confusables in source code would be far easier to consider malicious

  • mgaunard  3 years ago

    Except many people have a policy of ASCII only in source code and therefore would catch it immediately.

bschwindHN  3 years ago

Another point where I appreciate the Rust compiler:

    warning: the usage of Script Group `Cyrillic` in this crate consists solely of mixed script confusables
     --> src/lib.rs:1:4
      |
    1 | fn SYS_landloсk_create_ruleset() {
      |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
      |
      = note: the usage includes 'с' (U+0441)
      = note: please recheck to make sure their usages are indeed what you want
      = note: `#[warn(mixed_script_confusables)]` on by default

https://play.rust-lang.org/?version=stable&mode=debug&editio...

  • nebulous1  3 years ago

    The character was in a string, not directly in what was being compiled. The contents of the string failing to compile was the point, as landlock was then disabled.

    From what I understand, this landlock disabling wasn't relevant to the sshd attack. It appears it was setting up for something else.

kaashif  3 years ago

With the . the author can claim it was unintentional. It's impossible (or at least very hard) to claim the Cyrillic c is unintentional.

To me, that makes the . more evil.

dathinab  3 years ago

After the concept of such attacks had blow up idk. 1?2?3? years ago a lot of places now warn at least when mixing "languages"/ranges or sometimes on any usage of non us-ascii printable characters.

So stuff like that is increasingly more reliable caught.

Including by 3rd parties doing any ad-hoc scanning.

Through adding `compiles check` fail with syntax errors definitely should be added tot he list of auto scan checks, at least in C/C++ (it's not something you would do in most languages tbh. even in C it seems to be an antipatters).

Etheryte  3 years ago

Nearly every modern IDE and diff viewer instantly highlights this though? I doubt this would get far.

signa11  3 years ago

compiler would perhaps "see" it ?

  • josephg  3 years ago

    Sure; but so long as the compiler error is silently discarded (as it is here), the configure script will assume landlock isn't available and never use it.

    • smashed  3 years ago

      A misplaced punctuation has some plausible deniability. Like the author could say he was distracted and fat fingered nonsense, honest mistake.

      A utf character from a language that has zero chance of being mapped to your programmer's keyboard in the middle of a code line, that would be obvious intentional tampering or at the very least raise some eyebrows.

      4 replies →