[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.