[flagged]
Xz: Can you spot the single character that disabled Linux landlock?
221–230 of 322 posts
Re: Xz: Can you spot the single character that disabled Linux landlock?
#222Earlier quoted context omitted.
So that function checked if the following C code compiled, and only in that situation enabled the landlock? Except that lone period, hard to recognize because of its small size and proximity to the left edge of the diff, caused the C code to become always invalid, hence keeping the landlock always disabled? That's both vilely impressive and impressively vile. I didn't even spot it on my first read-through.
Even more evil would have been to replace this line (void)SYS_landlock_create_ruleset; with this: (void)SYS_landloсk_create_ruleset;
Nearly every modern IDE and diff viewer instantly highlights this though? I doubt this would get far.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#223Earlier quoted context omitted.
Maybe, eventually, but how many people read the reams of garbage autoconf spouts out until the feature they wanted fails to materialize?
The actual compiler output from the autoconf feature test is one of the things I'd probably look at fairly early on if some feature is disabled when it shouldn't be, but I maybe have a bit more experience running into problems with this than younger folks.
> if some feature is disabled when it shouldn't be
That's my point, you're going to look at autoconf's output if a feature you're expecting is missing.
But would you think to have a test or expectation for landlock in xz? How would you even check if landlock is enabled for a process?
Re: Xz: Can you spot the single character that disabled Linux landlock?
#224Earlier quoted context omitted.
Yep. It was a few years ago while that was stilled allowed (as I'd noted ;) ) but regardless. Significant whitespace is just annoying.. There's a lot of things that render as whitespace, and source code one might be reviewing could be printed wrapped or copied and pasted in odd ways. Other languages are more robust to this.
Never had this happen, this is largely eliminated by using tools like black or other autoformatters.
I feel that the main point still stands, though. Saying that Python doesn't have a whitespace problem because you can send the code through a tool that detects whitespace-related problems still acknowledges the existence of said problem.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#225Earlier quoted context omitted.
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.
It seems like there should be a way to catch these types of “bugs” - some form of dynamic analysis tool that extracts the feature detection code snippets and tries to compile them; if they fail for something like a syntax error, flag it as a broken check. Expanding macros on different OSes could complicate things though, and determining what flags to build the feature check code with — so perhaps filtering based on t…
I'd prefer if the ecosystem standardized on some dependency management primitives so critical projects aren't expected to invent buggy and insecure hacks ("does this strong parse?") in order to accurately add dependencies.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#226Earlier quoted context omitted.
It seems like there should be a way to catch these types of “bugs” - some form of dynamic analysis tool that extracts the feature detection code snippets and tries to compile them; if they fail for something like a syntax error, flag it as a broken check. Expanding macros on different OSes could complicate things though, and determining what flags to build the feature check code with — so perhaps filtering based on t…
> if they fail for something like a syntax error, flag it as a broken check. A syntax error might be exactly what they’re looking for e.g. they’re feature testing a new bit of syntax or a compiler extension. > so perhaps filtering based on the type of error would be best done as part of the build system functionality for doing the feature checking. Which would require every compiler to have detailed, consistent, and…
Universal support of SARIF by compilers would be most of that.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#227Earlier quoted context omitted.
Even more evil would have been to replace this line (void)SYS_landlock_create_ruleset; with this: (void)SYS_landloсk_create_ruleset;
For those squinting, the "landlock" regular "c" is replaced with a Cyrillic U+0441.
Yip, this was impressive. Only a copy & paste into VSCode (Windows) revealed the rectangle around the "c" in the second line.
Impressive.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#228Earlier quoted context omitted.
What does the dot do?
The function is “check_c_source_compiles”. The comment indicates that the intention is to confirm that the Landlock functionality can be compiled on the system, in which case it will be enabled. The stray dot isn’t valid C, so it will never compile. By ensuring it can never compile, Landlock will never be enabled.
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.)
Re: Xz: Can you spot the single character that disabled Linux landlock?
#229Earlier quoted context omitted.
You are quoting Jia Tan [1]. The malicious actor wrote that comment when deliberately breaking the check in the first place. Fixing headers or extra tests would not have prevented this, as there is no indication the headers were broken in the first place, and extra tests could have been compromised (or ignored for release tarball) some other way. [1] https://git.tukaani.org/?p=xz.git;a=commit;h=328c52da8a2bbb8...
Should the better fix then to have been to revert the bad commit with the malicious commit message, rather than just deleting the dot (as was done)?
The better fix would be to move to proper dependency management and depend on a version range of a dependency instead of hoping you can do a better job of modelling the same with strings hardcoded into a CMakeLists.txt.
Re: Xz: Can you spot the single character that disabled Linux landlock?
#230Earlier quoted context omitted.
I think main issue was that it was embedded in the file itself like that. Would have preferred to have it in a separate valid C file with syntax highlighting etc and being parsed from that file.
Perhaps, but given how most build systems work, that would complicate things in other ways (since build systems often try to compile all .c files).
Put it in a special folder that is ignored from the build.