[bob1029]

I am curious how this code actually made it in, based upon the following:

> The fact that the compromised file is digitally signed suggests the attackers were able to access the company’s software development or distribution pipeline. Evidence suggests that as early as October 2019, these attackers have been testing their ability to insert code by adding empty classes.

Unless this a compromise of the build machine, it sounds suspiciously like a lack of code review standards to me.

In our organization, the only way to get a line of code into master is through a process where a 2nd developer reviews and approves via GitHub. Branch protection rules are really nice for this kind of concern. Obviously, the attacker can hit right after cloning source, but it helps to know your foundations are clean regardless.

[rossjudson]

All you need is to be able to influence the behavior of the build system at runtime. If you have that, you do not need access to the source code, and you do not need to check anything in. This includes scenarios where the tool chain itself is checked into the source control system.

Ken Thompson described this a long time ago: https://www.win.tue.nl/~aeb/linux/hh/thompson/trust.html

At runtime, you alter the build system to notice if a new compiler binary is being built. If so, inject code that, from then on, invisibly "injects the injector". Once you've shifted to this new compiler, the compiler binary itself is the attacker.

Since you have an injector, you can put other types of detections and alterations in...like watching for the compilation of the initialization of a Solarwinds DLL. Then you inject in what you need. No source code is involved.

There are probably lots of ways of getting around this, but unless you're actually looking for it, you won't see it.

[saagarjha]

Successfully pulling off such an attack, of course, is quite difficult.

[diamondo25]

Nothing is difficult if theres a good reason to do it and you have money and time to do the attack :)

[whoisthemachine]

This is really interesting - really seems to be an argument in favor of all code being open, shared source - along with deterministic builds, it would be easy enough for any organization to build Orion themselves and verify they get the correct build hash. Or if there was a disagreement in hashes between SolarWinds and their imagined community, it would serve as a red flag.

[mikewarot]

This code was added to the Solar Winds source codebase, if I read the article correctly. No check in the world is going to catch it if it's actually in the base code.

Carefully compiling a corrupted program doesn't fix it.

[Shank]

If you view GitHub as a static infallible source, then yes -- your analysis is correct. But there are gaps anywhere. If you want perfect 1:1 mappings between source, the developers who make it, and the end builds, you essentially need a "chain of trust" that can be tested at every stage. For example: are all developers pushing code with encrypted SSH keys? Are the commits signed? Are the signing keys hardware backed? Does CI check the signatures? Is the CI server up to date? Are all packages on the CI server signed and trusted? Are all stages of the build pipeline testable for tampering and tamper-proof? You're not curling or apt-getting or running npm anywhere in your build server for some kind of Slack integration, right? The list goes on.

The issue is that most developers view "the code pipeline" as a trusted and complete system, for the most part. In the vast majority of cases, that's okay. The issue is that SolarWinds should have known, based on their very own customer list, that they were in an advantageous position in many organizations that are valuable targets. That should have _caused_ all of this thinking to happen, and led to changes internally to accommodate the new risk. That threat modeling/analysis either didn't happen, or the outputs weren't good enough.

[blastro]

very well articulated, thanks.

[eternalny1]

They are claiming that their build system was compromised and the code was not under source control.

> Based on our investigations to date, which are ongoing, we believe that the vulnerability was inserted within the Orion Platform products and existed in updates released between March and June 2020 (what we call the “relevant period”) as a result of a compromise of the Orion software build system and was not present in the source code repository of the Orion Platform products.

[ashleyn]

In my experience and where I work, the build system tends to be the most neglected part of the pipeline, most trouble-prone and frequently the source of headaches nobody wants to bother with. I think the days of build being the red-headed stepchild nobody wants to deal with is coming to an abrupt end.

[dboreham]

Yarn cache? (or similar bogus feature in other language tooling).

[nwellinghoff]

Spoken like a true dev There are other ways of checking code in besides the official channel. Almost every company on the planet could fall victim to this type of attack. Once a team gets past a certain size and “its not my job” comes into play. All kinds of doors swing open.

[alkonaut]

It’s too obvious even for a huge app. The empty catch alone is something I’d immediately “git blame” if I saw it. I work on a 20 year old massive enterprise app and there is lots of “not my job”, but someone would see it.

Also, it would likely (or hopefully) trigger a static analysis warning in the build as soon as it’s added. For such a sophisticated attack this would be too much of a weak point. It would be much better to have access to a point in the build system that enabled you to inject that code in or after the compilation, e.g by tampering with the tool chain on the build machines.

[toyg]

The issue here is that the code was added at build time. Do you do code-reviews after decompiling build output? If you’re a sane person, probably not - so you’d fall for this too.

An automated tool might have more of a chance, but again it’s kinda hard to have one that runs on binaries. If it runs after build, it’s typically some input/output checker, which would not detect code like this.

It’s a hard problem and I think it has just demonstrated that the security of build-related infrastructure should be taken more seriously than it currently is.

[alkonaut]

Yes, exactly. Has to be added to the binary or after static analysis of the source. Adding it too the source would too easily risk discovery.

Even if I did have source write access, rather than adding the poison to the runnable code, I’d add the poison to code run at build time (a unit test) which modified the build tool chain and then removed all traces of the poison code again.

[slaymaker1907]

I was thinking something similar. My first comment on review or even looking at the code is why the exception is swallowed up without logging or a comment about why there is no logging.

That of course leads one to ask what this code is actually doing.

[tsimionescu]

If we're at the level where we think it's an inside job, it doesn't seem that difficult to have 2 people on the inside "reviewing" each other's malicious commits.

For what it's worth, my org also has the same policy, but it's intended to catch mistakes, not to protect against malicious actors inside the company.

[Spooky23]

The vast majority of software shops don’t even consider insider threat in any meaningful way.

Imo it’s would be trivial to compromise many. Most companies have soft underbelly units like offshore maintenance engineering, tools teams and patching teams who don’t get a lot of meaningful oversight and can bypass many controls.

[saagarjha]

I mean, not even that. The cost to buy a software engineer and get them hired at the place you want to attack is really not that high. Once inside it’s generally possible to get things in (“the guild server was failing so I SSHed in and fixed it”).

[dboreham]

Whispers for decades that NSA does this.

[netsharc]

Or have them compromised? In the spy movies they'd send a female agent to seduce and then blackmail the married high ranking official, a scenario that bachelor software developers probably dream of.

[phendrenad2]

Do you code review the output of your build system? Imagine you had to write Java for a minute. Are you going to open up JAR files and look at strings to ensure no one is inserting code from your build server?

Or if Java is a bridge too far, are you inspecting your minified webpack output to ensure no one is inserting malicious Javascript?

[Terretta]

> Unless this a compromise of the build machine ...

Assume compromise of the build machine. Start with, who builds the build machine, and how do they maintain it? Can humans get into it at all, such as in a break glass scenario?

Also, GitHub, GitLab, and the like, may not actually guarantee what you are relying on to enforce “the only way”.

[newhouseb]

Solarwinds accidentally leaked (via Github) the FTP credentials to the infrastructure used to distribute builds in late 2019 [1].

I'd be curious to see if the digitally signed bad versions are similar to digitally signed good versions, i.e. if there's any chance the attacker found/developed a hash collision against an otherwise legitimate build. AIUI it'd be a pretty big deal since it would point to a vulnerability in SHA-256 (which is usually how Windows binaries are signed), but this is apparently a nation state we're dealing with? ¯\_(ツ)_/¯

[1] https://twitter.com/vinodsparrow/status/1338431183588188160/...

[rurban]

They did sign it with the key they found there, virus vendors detected fancy bear or such, customer support was in denial and recommended all customer to ignore this warning and disable scanning this binary, whitelist.

You don't need the GRU with such a company. Microsoft Defender would not help. Even a 12 year old from mom's basement could have intruded the nuclear arsenal this way. The nation state allegation came from the stealth CC stuff they found. But apparently someone else also took the invitation via writable ftp.

[mekkkkkk]

Branch protection settings are also editable by someone. What's to say the attacker couldn't disable it or bypass it for this one commit? Also I don't think it's uncommon to have a way to get a hotfix deployed without going through the normal checks and balances. For those "the service is down" calls at 2 AM.

[maccard]

I've said this before, but I work on a team of 5 or 6 people. If I (pre covid) sent them a PR and walked over to their desk, told them it was super urgent and a tiny change just needed a rubber stamp, one of them would do it (and I would likely do the same for them). Failing that I can name a handful of developers that wouldn't be familiar with the system but will review my change because I did the same for them a few months ago (and they'll comment on stylistic/clarity issues, rather than the work being done). Even if you think this is rare, it likely isn't and likely happens at every company to some degree.

[yenwel]

I've worked at a mediacorp in user authentication team where one rogue junior developer from another team (with the most seniority though at that company subdividion because everybody else left) went behind my back to pressure my junior colleague to merge a pr in our codebase which opened a security hole in the back end because he was working together with a project lead who promised to deliver something that we couldn't.

[maccard]

Exactly. Even if it didn't have a security issue, one developer going to a trusted dev on another team and saying "hey, it's the week before Christmas/9pm on a Friday night, nobody else is here, I really need this merge, it's low risk and can go out in the next rollout but QA need to check it this weekend/I'm off next week/<insert some business reason applicable here>" will often result in a thumbs up.

[macNchz]

Code review and protected master are certainly important but not infallible. If I were a malware author with code running on an owned dev machine and my goal was to sneak code into a repo, I can think of a bunch of strategies that might increase the odds of slipping past a review.

Just running in the background, waiting to amend a big commit with many changed files/lines would probably go a long way. How often does a reviewer glaze over when reading through a diff where someone shuffled some modules around, causing a lot of line changes without any real implementation changes? Perfect opportunity to slip a few new lines into a long file amidst all the other changes.

[netsharc]

As many are saying here, why commit the malware, just modify Jenkins to detect that it's compiling this DLL and add those lines into the source about to be compiled..

[dboreham]

I offer up a glass of kool aid if you believe this works. Employee A is just going to DM slack Employee B asking them to approve their 7000 like PR because they're going on vacation next week and Employee C has been slow reviewing.

[raverbashing]

I'd be even more curious to know why people saw that code and thought nothing of it.

No git blame? No bugs around that area? No one questioning what/how/why is that there?

[codenesium]

They likely put a program on the build server to make the changes after checkout before build. Which would be impossible to detect.

[raverbashing]

"impossible to detect" except for all the merge conflicts and build breaking changes, etc

[vsareto]

Huh? After the build server checks out the code, write malicious code to the source files directly just before the compilation step. No merge conflicts or breaking changes unless the added code failed to compile. It's reasonable to guess they had access to the source from the build server, so they could reproduce this environment themselves and test on their own.

[mehrdadn]

It's not hard to envision code reviews that don't review every single line. Been on both sides of it. Code review isn't a security barrier, it's a (noisy) safety check. It can't even catch every silly bug, let alone deliberate covert sabotage.

[sorokod]

The chunk of code that unzips the Base64 encoded strings looks super dodgy and should jump right out even at a casual reviewer.

[mehrdadn]

Maybe, assuming it's all committed in one shot (rather than, say, a couple lines per review) and the reviewer scrolls through and glances at every chunk of code. It's some 4000 lines of code, so I wouldn't be surprised if at least one of those wasn't the case. (And all of this is assuming the reviewer was a typical one... if they were particularly known to be careless or compromised, then you don't need these either.)

P.S. Oh, one more thing: some workflows don't mandate a second review after the code is amended, so the initial code could be benign. And many don't require the merger to be the reviewer either. So this could've gotten in that way too.

[chris_wot]

Can’t you just run a git rebase to disguise it?

[saagarjha]

Depending on what you rebase to and what the usual workflow is for developers. Those that rebase from remote frequently (guilty) won’t notice, but those that do merges might.