12 ms·
I'm a maintainer (one of many) of an open source project, and this topic has been on my mind a lot lately as I review PRs. I am more suspicious of PRs from new
by bsuvc 2y ago
I'm a maintainer (one of many) of an open source project, and this topic has been on my mind a lot lately as I review PRs.
I am more suspicious of PRs from new contributors by default now. Of course I keep these suspicions to myself, but besides simply reviewing code for all the regular things, I now ask myself "what sort of sneaky thing could they be doing that appears benign on the surface?"
- andy99 2y agoIt's not the new contributors you have to watch, it's the sleeper contributor who has built up a solid reputation and then is "activated". At least that's how I understand XZ.
- Uehreka 2y agoIt’s both. The fact that one happened recently does not preclude the other.
- ikekkdcjkfke 2y ago"Trusted account for sale"
- andix 2y agoThat's great that you are considering this more now. But the xy story taught us, that every contributor is dangerous, the most dangerous ones are probably the most helpful and most skilled contributors. If someone barely get's a PR accepted, they probably lack the skills to add a sophisticated backdoor. Another thing that was not talked about a lot: There are many ways to compromise existing maintainers. Compromising people is the core competency of intelligence, happens all the time, and most cases probably never come to public knowledge.
- andix 2y agoOne follow up to compromising existing maintainers: This makes the creators or long-term good faith maintainers maybe even more "dangerous" than new maintainers.
- WanderPanda 2y agoAre we facing a Byzantine generals kind of situation now?
- Mtinie 2y agoWe have always faced it, it’s just that there's more awareness of the potential issues.
- tschwimmer 2y ago>If someone barely get's a PR accepted, they probably lack the skills to add a sophisticated backdoor. Unforuntately it's easy to sandbag being dumb. Just because someone submits a PR defining constants for 0-999 does not mean they're actually bad at programming.
- andix 2y agoSure, but being known for submitting bad code is going to make code reviews more thorough, not less. It's drawing additional attention to yourself.
- MenhirMike 2y ago> defining constants for 0-999 That person might just be an old school Java <5 developer.
- chipdart 2y agoThat person might just be a regular Java developer who works on a project which onboarded Checkstyle, and can't disable it's MagicNumber check. https://checkstyle.sourceforge.io/checks/coding/magicnumber.html https://checkstyle.sourceforge.io/checks/coding/magicnumber....
- raxxorraxor 2y agoMan, I hate such tools. Do I run into problems when I try to convert seconds to minutes? Larger problem than magic numbers ever could be.
- MenhirMike 2y agoYes! Anytime you see a function signature like "int timeout", it's safe to assume that the unit is in femtoseconds and pass a gigantic number while you curse out the incompetence of the developer. Either name your variables correctly (timeoutZeptoseconds), or use a proper data type (like a Duration or Period in Java, TimeSpan in C#, or a user-defined literal in C++).
- victorbjorklund 2y ago> Compromising people is the core competency of intelligence, happens all the time, and most cases probably never come to public knowledge. Yea. It would almost be strange if security service didnt consider the route of getting "kompromat" on a developer to make them "help" them.
- andix 2y agoThey would be really bad at their job, if they didn't try.
- transpute 2y agoDevelopers would be really bad at their newly expanded job, if they didn't resist.
- MichaelZuo 2y agoThe most secure systems are those that are also resistant to rubber hose cryptography.
- nbk_2000 2y ago"Rubber Hose Cryptography" comes in the form of a PR. "Rubber Hose Cryptanalysis" comes in the back door and waits for you in the dark.
- MichaelZuo 2y agoNo, it 'comes in the form of' a rubber hose...
- nyokodo 2y ago> consider the route of getting "kompromat" on a developer to make them "help" them I suppose that’s an option, but it also introduces an additional risk of exposure for your operation as it doesn’t always work and makes it much more complicated to manage even when it does work.
- arp242 2y ago[flagged]
- Der_Einzige 2y ago[flagged]
- deleted 2y ago[deleted]
- nineteen999 2y agoFlipside of that being a highest number of school shootings in the world.
- jrockway 2y agoI don't think the "armed to the teeth" theory is correct. If you were right, people wouldn't honk at each other or otherwise involve themselves in any sort of road rage. But people rage at each other all the time, and only very rarely does someone get shot. The reason people aren't walking around stabbing you in the eye with a needle is because there is no reason for them to do that. They gain nothing. They don't desire that it be done.
- emodendroket 2y agoIf the news articles about Instagram extortion are anything to go by, adding weapons to an extortion situation is more likely to lead to a suicide than the extortionist being dissuaded.
- andix 2y agoWe know about the failed attempts, we have no idea about the successful ones, and the ones that are going to be successful in the future.
- arp242 2y agoYou can always use this line because you can never prove something doesn't exist. Go find evidence. It's been over a month.
- xpe 2y agoWho can share a threat model with specific probability estimates on this? FWIW, I’m less interested in the particular estimates (priors) and more interested in the structure.
- mavelikara 2y ago> Another thing that was not talked about a lot: There are many ways to compromise existing maintainers. Also not talked about a lot - there are many ways to compromise existing software engineers who are paid to work on proprietary software systems.
- bee_rider 2y agoIt is a worthwhile reminder. But, source-not-available proprietary systems are just totally hopeless from this point of view, of course an intelligence agency could slip something on. A bored developer at the company could too. Users of this sort of proprietary system have just chosen to have 100% faith for some incomprehensible reason.
- spopejoy 2y agoI don't think that's relevant to this discussion though, as open- and closed-source subversion would seem to follow really different paths. - Open-source subversion has the big advantage of having the code, testing and build processes in the open which allows for the attack surface to be exhaustively studied, whereas closed source requires code exfil, reverse engineering, inside intel on processes etc. - Closed-source subversion can hide in other places -- binaries can be corrupted on a compromised server etc. Seeking to influence the code-based development seems like the hardest road IMO. - Open-source maintenance (at least the kind under discussion here) stops at the maintainer, whereas most corporate dev is in a hierarchy with non-uniform commit authority. None of the same social techniques would apply.
- lupusreal 2y ago> If someone barely get's a PR accepted, they probably lack the skills to add a sophisticated backdoor. That's true, but it's also true that a sophisticated and well formed PR is probably genuine too. Hostile PRs are the exception rather than the rule. And if only the high quality PRs are treated with suspicion, then the attackers will tailor their approach to mimic novices. General vigilance is required, but failure is likely because these attacks are so rare that maintainers will grow weary of being paranoid about a threat they've never seen in years of suspicion and let their guard down.
- lytefm 2y agoEarly this year, I've received a hostile PR for a "maintenance only" JavaScript authentication library with less than 100 stars but which is actively used by my employer. It added a "kinda useful but not really needed" feature and removed an unrelated line of code, thereby introducing a minor security vulnerability. My suspicion is that these low quality PRs are similar to the intentional typos in spam emails: Identify projects/ maintainers who are sloppy/ gullible enough and start getting a foot in the door.
- runjake 2y agoLooking at some of these cases, each PR on their own doesn’t look suspicious, but it was what they all built up to — in some cases from multiple bad actor contributors that, on the surface, weren’t connected.
- WanderPanda 2y agoWasn't a key thing of the xz attack vector that people where encouraged to download the custom source release instead of the autogenerated Github one? I don't know if that is a pattern but it seems like best practices in the (source) supply-chain could prevent a large class of these attacks.
- leeoniya 2y agoyep. same with npm. i publish releases of my OSS libs to npm, but there's no guarantee that what is uploaded is what you see on github. that's a lot of trust you have to put into my opsec, etc. not good.
- supriyo-biswas 2y agoThat is unfortunately how `the `autotools` ecosystem works; although I guess projects could guide their users to run `autoreconf -i` if working with the source code instead of the release tarballs before doing the usual `./configure && make && make install` step.
- jowea 2y agoCan't you just commit the configure file?
- out-of-ideas 2y agothe attitude remindes me of maintaining game-servers and looking out for cheaters; once we had a handful of folks looking out for cheaters, it turned the community against itself calling everybody a cheater... i think it is good to be cautious; but overall it's the same cat and mouse game we've seen before. i can only say good luck on not letting it stress you out second guessing other folks actions and intent - and hope we continue writing code for humans to read vs the cryptic, obstrufcated, even "elegant" code (not to dive into the skill issue rabbit hole lol)
- SlightlyLeftPad 2y agoI’m in it for a free t-shirt.
- arccy 2y agohttps://medium.com/pentesternepal/hacking-dutch-government-for-a-lousy-t-shirt-8e1fd1b56deb https://medium.com/pentesternepal/hacking-dutch-government-f...
- Beefin 2y agowhat do you look for to prevent this?
- euroderf 2y agoWhy is there not a policy that any PR can be rewritten by a maintainer ? Wherever the PR looks a bit odd, rewrite it so do the same thing a different way. Enough unpredictable change to disrupt finely-tuned subterfuge.
- vincnetas 2y agoyou can wait for tree (or x) PR's passing specified unit tests for functionality and then merge a random one. But this is a luxury (effort wise) for any kind of project.
- Grimeton 2y agoThis is what I feared the most. Trust issues that lead to less progress and a community that slowly drowns in suspicions. When the XZ thing happened they were already going after one person accusing them of being part of the whole thing in one of the bug reports on github.
- sesm 2y agoHonestly, a good PR should have a very clear description of the idea and a sample implementation, and then a trusted core contributor re-implements the fix on his own. But Github users are entitled and spoiled by Github-marketed commercial software, so they will rage at this.
- 1231232131231 2y agoSounds like you're describing an issue, not a PR.
- sesm 2y agoAn issue usually doesn’t have the code for implementation of the solution. Yes, very often patches are attached in comments, but they are not required and usually attached by other people, not the author.
- PurpleRamen 2y agoThis should be the mindset for any commit. I mean, they could add something benign unknowingly. Something could hack their account, or abuse a flaw in your commit-system to appear as someone else. They could have a melt-down, or strange ideas and adding something for nonsical reasons. Shit can happen all the time from all directions for any reason.
- 1vuio0pswjnm7 2y agoConsider this comment: "But the xy story taught us, that every contributor is dangerous, the most dangerous ones are probably the most helpful and most skilled contributors." What is "xy"? It's tempting to classify this as a typo but y is a long way from z on the keyboard, while the x and z keys are adjacent. I type xz on a regular basis because it is so often used in place of gz for compressing tarballs. I cannot imagine calling it "xy" unless I rarely used it.
- noashavit 2y agoIt’s not the new contributors, but older ones that might have already built a rapport with you so you are less critical in your review. That what happened at Xz utility and the hacker nearly got access to every Linux machine out there.