Recently, a security-related bug slipped into libcurl 7.52.0.
For those of you who don’t know, libcurl is a popular open source library that supports many protocols and greatly simplifies data transfer over the Internet; an uncountable number of open- and closed-source projects depend on it.
Because of the bug, this particular version of libcurl doesn’t use random numbers when it should, which is really bad for security:
|
1 2 3 4 5 6 7 8 |
static CURLcode randit(struct Curl_easy *data, unsigned int *rnd) { // ... 24 lines ... result = Curl_ssl_random(data, (unsigned char *)&rnd, sizeof(rnd)); //... } |
Since all the surrounding code is stripped away it is pretty easy to see what went wrong, right?
Within ‘randit’ there is an attempt to obtain a random number by calling ‘Curl_ssl_random’. However, ‘Curl_ssl_random’ is not passed the pointer ‘rnd’, but instead a pointer to ‘rnd’. Hence, the memory pointed to by ‘rnd’ is not filled with a random number but rather the pointer ‘rnd’ will point to a random memory location.
How did this bug come about? I’m pretty sure that — initially — the unlucky developer had accidentally typed this:
|
1 2 3 4 5 6 7 8 |
static CURLcode randit(struct Curl_easy *data, unsigned int *rnd) { // ... 24 lines ... result = Curl_ssl_random(data, &rnd, sizeof(rnd)); // ... } |
When (s)he compiled the code with gcc, the following error message was produced:
|
1 2 3 4 |
rand.c:63 error: cannot convert ‘unsigned int**’ to ‘unsigned char*’ for argument ‘2’ to ‘CURLcode Curl_ssl_random(void*, unsigned char*, size_t)’ |
Which exactly explains the problem, but most likely, the developer only skimmed the error message and jumped to the wrong conclusion; that is, (s)he thought that a cast was needed because of a simple pointer incompatibility (unsigned int* vs. unsigned char*) when in fact there is a severe pointer incompatibility (pointer to pointer vs. pointer).
I’ve seen this many times before: developers apply casts to get rid of warnings from the compiler (or a static analysis tool) without a second thought. Don’t do this. Be very considerate when your compiler speaks to you. Casting, on the other hand, will silence it forever.