CodeSOD: Secure Cryptography

Governments have a difficult relationship with cryptography. Certainly, they benefit from having secure, reliable and fast encryption. Arguably, their citizens also benefit- I would argue that being able to, say, handle online banking transactions securely is a net positive. But it creates a prisoner's dilemma: malicious individuals may conceal their activity behind encryption. From the perspective of a state authority, this is bad.

Thus, you get the regular calls for a cryptosystem which allows secure communications but also allows the state to snoop on those messages. Of course, if you intentionally weaken a cryptographic system so that some people can bypass it, you've created a system which anyone can bypass. You can't have secure encryption which also allows snooping, any more than you can have an even prime number larger than two.

This leaves us in a situation where mathematicians and cryptography experts are shouting, "This isn't possible!" and cops and politicians are shouting "JUST NERD HARDER!" back.

Well, today's anonymous submitter found a crypto library which promises to allow secure communications and allow nation states to break that encryption. They've nerded harder! Let's take a look at some of their C code.

unsigned long int alea(void){ FILE * f1 = 0; unsigned char val = 0; /*float rd = 0;*/ unsigned long int rd = 0; f1 = fopen("/dev/random","r"); fread(&val,sizeof(unsigned char),1,f1); /*rd = (float)(val) / (ULONG_MAX);*/ rd = (unsigned long int)((val) / (UCHAR_MAX)); fclose(f1); return rd;}

This function reads a byte from /dev/random to get us some random data for key generation. Unfortunately, it's using the XKCD algorithm: this function always returns 0. Note that they divide val by UCHAR_MAX before casting val to an unsigned long int. Which, there's also the issue here that 8 bits of entropy are always going to be 8 bits of entropy- casting it to an unsigned long int isn't going to be any more random than just passing back the 8-bits, because you only read 8 bits of randomness. There's also no reason why they couldn't have simply read an unsigned long's worth of random data.

I appreciate the comment which indicates that rd used to be a floating point number. It doesn't make this code any better, but it's nice to see that they keep trying.

Let's also take a peek at a use-after-free waiting to happen:

void MY_FREE(void *p){ if (p == NULL) return; free(p); p = NULL;}

They wrote their own MY_FREE function, which adds a NULL check around the pointer- don't free the memory if the pointer points at NULL. Nothing wrong with that (though this is better enforced structurally with clear ownership of memory, and not through conditional checks, so actually, yes, there's something wrong with that). After we free the memory pointed to by the pointer, we set the pointer equal to NULL.

Except we don't. We set the local variable to NULL. So when the code does things like:

 if(v->aliveness == NULL) { MY_FREE(v); return(v); }

v is a pointer to our struct. If the aliveness value is NULL, we want to delete that data from memory, so we call MY_FREE, and then we return our pointer to the struct- which is unchanged and definitely not null. It's pointing to what is now freed memory. If anyone touches it, the whole program blows up.

Here's the upshot: it's almost a guarantee that this program has undefined behavior in there, someplace. This means the compiler is free to do anything, include implement a dual custody cryptographic algorithm which is magically secure and prevents abuse by state actors. It's as likely as nasal demons.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!