Comment by dhx

This is what the cargo/rust ecosystem does :)

It's standard autoconf stuff, BUT... the right thing to do for a security option should be to throw an error at the configure stage If the Option is requested but can't be supported, And not to silently turn it off.

That is because a human should have to Look at the build and manually make the decision to switch off a security feature from the build.

Agreed. I just opened https://gitlab.kitware.com/cmake/cmake/-/issues/25846 although I have no idea how hard it is to address this sort of issue robustly.

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

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

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...

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.

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).

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

compiler would perhaps "see" it ?

Yeah, if anything, python worsens the situation. I had a friend DOS our server because he accidentally inserted a tab, causing the illusion that one statement was inside a block but was actually outside it. He swore off python at that point. I personally avoid the language, but I understand due to issues like that these days mixing tabs and spaces is an error (or is it just a warning?) by default. Regardless, still pretty silly to me to have whitespace play such a major significant role, besides the fact that I find it visually harder to read, like text without punctuation or capitalisation.

In Python, you only need to indent the `def test_foo` by an additional whitespace, to make it a locally scoped function instead of a proper test function.

AST diffs instead of textual diffs might have helped here (to spot the `.` making the code un-compilable).

Edit: oof, though the stray character in question is inside a perfectly legitimate C string, so to catch this, any such diffs would need to Matroyshka down and that seems unsolvable / intractable.

I'm not sure that an IDE will catch a syntax error in C code quoted inside a cmake script trying to test if things compile.

Ok there's a larger question here about the bazaar software development method.

How can we ensure say, that Microsoft doesn't pay someone to throw a wrench in libre office development or Adobe to sabotage Gimp?

There's lots of deception strategies for bad faith actors and given the paucity of people who actually do the work, it's really really hard to be picky.

Especially with the complexity of library dependencies. Defect-at-a-distance may actually be a common strategy and this is the first that was caught

I'd never suspect this to be intentional if I'd spot it in a patch, even given the consequences in this particular case. I have written and committed things into code instead of writing it into some other window several times. Without a linter I probably wouldn't spot an extra dot when reviewing my own change before sending it out.

I love this type of hindsight to 20-10 comment. "If I saw...". That is a BIG if. Plenty of smart people on that mailing also missed it. I missed it myself when I opened the HN lead link. Very subtle.

I distinctly remember having to remove such a superfluous . that I accidentally added into a file on multiple occasions.

If you are using vi and your left shift key is dodgy, that can easily happen by accident.

Not me. Maybe someone is using an editor with a . key mapped to something. It's in a pretty convenient place.

:wq!

Double spacebar gives you period in some contexts on Mac.

Truly seems that way currently. He said he'd really dig in starting next week and just checked his email on vacation and saw this whole mess.

I love unified diffs for many applications. You are right that side by side diffs have their uses, too.

Yeah, but I’m wondering where the code review was done. In a different context, would this be easier to spot?

Shouldn't there be a unit test to confirm landlock is on/off? (I mean, this seems a crucial aspect of the code which needs 100% test coverage.)

configure would print that it's not enabled, so it seems like the kind of thing people would eventually notice.

It feels like this is a poor way to determine if a feature should be enabled. There are just too many reasons it could fail, and default off on compilation failure just seems like a glaring security bug. If this is a common practice, it seems like the community should rethink this choice.

I don't think it fails the build. It's part of a test, trying to compile a bit of code. If it compiles the test is true and a certain feature is enabled. If the compilation fails the the feature is disabled.

This is quite common in build systems. I had such a test produce incorrect results in the kernel build system recently. The problem is that the tests should really look carefully for an expected error message. If the compilation fails with the expected message the test result is false. If the compilation fails for some other reason the build should fail so the developer is forced to investigate.

Disclaimer: I have not studied the xz build system in detail, just what I saw from the diff plus a bit of extrapolation.