11

I'm using the following code:

AppDomain.CurrentDomain.AssemblyLoad += (sender, args) =>
{
    var token = args.LoadedAssembly.GetName().GetPublicKeyToken();

    if (!IsValidToken(token))
    {
        Process.GetCurrentProcess().Kill();
    }
};

Where IsValidToken() compares the public key token of the assembly being loaded against a list of authorized public key tokens hardcoded in my application as byte arrays.

Is this a good security measure to prevent code injection attacks? Also, is this necessary given the fact that I will later obfuscate my application using NetReactor? I'm trying to prevent any "snooping" into my application, not only coming from the Snoop tool, but from any external undesired sources as well.

Daniel A.A. Pelsmaeker
  • 47,471
  • 20
  • 111
  • 157
Federico Berasategui
  • 43,562
  • 11
  • 100
  • 154
  • 2
    You'll have a hard time trying to figure out the perfect and future-proof authorized list of valid tokens. You may have undesired side-effects for your valid users (like the problems with copy-protected CDs that were a problem only for those who bought it...). At the same time, a good hacker will disassemble the whole thing, and just patch that code with nops. – Simon Mourier Mar 05 '13 at 11:16

2 Answers2

3

Just from first glance, I'm going to say "no, this won't be enough".

Reasons:

  • CreateRemoteThread attacks are straight win32 calls, no managed code traces that would trip a detector like this

  • I think it would be possible to create another AppDomain in the injected dll, thus bypassing this check altogether. Then one could execute code from that AppDomain, potentially (I'd have to think that through) calling back into the "main" AppDomain via AppDomain.DoCallback

  • Process.Kill is a horrible way to drop your application, although it is a non-trappable way of doing so - that is, anyone attached wouldn't be able to prevent it (it uses Win32 TerminateProcess under the hood)

I'd have to bust out my "Injecterator" harness to test these statements, tho - if I can remember where the heck I put that code...

Regardless of any of these - you will absolutely want to obfuscate the hell out of this assembly, especially if you plan on storing sensitive bits inside (in fact, I'd argue against storing ANY sensitive information inside an assembly if you can help it) - your prevention method will absolutely NOT stop any disassemblers like Reflector, ILSpy, dotPeek, etc.

JerKimball
  • 16,584
  • 3
  • 43
  • 55
  • What would you recommend as extra security measures? beside obfuscation (which I am already doing) to prevent injection (that causes an application to have unwanted behavior) and intellectual property protection? – Federico Berasategui Mar 05 '13 at 01:50
  • @highcore that depends a lot on your deployment scenario, and what your app is. – JerKimball Mar 05 '13 at 01:57
  • This is part of the `Core` class in my Client/Server application framework, which contains common infrastructure shared by the WCF server part and the WPF client. Applications developed using my framework will be deployed as client/server applications in a LAN environment in my customer's facilities. – Federico Berasategui Mar 05 '13 at 02:01
  • Hmmm...just thinking out loud, but on the managed side, you could trap the Assembly load event and reject anything that wasn't gac' ed/strong named...but preventing injection routes is a different beast. I'd have to think about that - I'm usually dealing with the greyer side of that equation. :) – JerKimball Mar 05 '13 at 02:05
2

It would also be safer if you generated the full keys at runtime (perhaps from multiple partial keys). This works around statically examining your binary for keys.

Mark Leighton Fisher
  • 5,609
  • 2
  • 18
  • 29