Is dereferencing a null pointer in C a security risk if the program isn’t a daemon, but a small script lauched as a separate process for each request?

Question

The following code is part of a program that is spawned at every request by the nginx’s ruby on rails script :

static void time_t_to_dos_time(time_t user_supplied_time_t, int *dos_date, int *dos_time)
{
    struct tm *t = localtime(&user_supplied_time_t);

    *dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;
    *dos_date = t->tm_mday + (t->tm_mon + 1) * 32 +
        (t->tm_year + 1900 - 1980) * 512;
}

localtimereturns 0 if the value is too large to fit in astruct tm. So when the program tries to readt->tm_sec, it will attempt to read memory address 0.
In that case, the program immediately raisesSIGSEGVand the server returns :

HTTP/1.1 502 Bad Gateway
Content-Length: 13
Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'
Strict-Transport-Security: max-age=31536000
Vary: Authorization,Accept-Encoding
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-XSS-Protection: 1; mode=block
Date: Tue, 28 Jun 2016 12:59:10 GMT

502: Failure

It appears to me to be a simple bug without any security concerns at all as the program is designed to only run on that website.
Would this be correct ?

What I would like to know is if it is definitely not a vulnerability. Please note the process don’t create any file. — user2284570, Jun 28 '16 at 14:20
Do errors take time? Does it dump core and take up disk space? — Neil Smithline, Jun 28 '16 at 15:31
@NeilSmithline : no core dump on ubuntu *(anyway if there were any the file wouldn’t be reachable from internet)*. and it takes near no time comparing to a search request *(which run on the same virtual hardware)*. — user2284570, Jun 28 '16 at 15:58
I think that "security risk" depends very much on your perspective. That said, why *not* add a simple `if(t != NULL)`, or at least add a `if(t == NULL)` and *handle* the error instead of relying on undefined behavior? Remember that out-of-bounds memory access is undefined behavior in C, so *anything* is technically a valid answer to "what will happen?". With a simple guard condition, you can *know* what will happen. That `cmp` and `je`/`jne` is going to be completely inconsequential, and even more so if (most likely case) the code is short enough that the jump doesn't cause CPU cache eviction. — user, Jun 28 '16 at 21:58
@MichaelKjörling : I don't maintain the program *(but yes I know how to fix this)*. It´s about on whether reporting it : https://support.hackerone.com/hc/en-us/articles/208041076-What-are-Signal-Requirements- . if there aren’t any argument on what risk it could involve I will be banned form making further reports. So if it’s definitely not a vulnerability I won’t report it. — user2284570, Jun 28 '16 at 22:04
"no core dump on ubuntu" - actually that should be "no core dumb on the curernt configuration of your server". A different config could have its filesystem filled up with core dumps on demand. But core dumps are rather useful things for a live server. — symcbean, Jun 29 '16 at 08:22
@MichaelKjörling, I agree. The one word I would change is perspective, because risks are ideally objective probabilities and costs that we may try to intuitively assess or numerically estimate. I'd say that, "Security risk depends on [the system] context." ... What I really like about your comment is your point that undefined behavior is generally undesirable in most contexts. Why not KNOW what will happen if it takes a simple test that compiles to just one CPU instruction in this case? :) — Douglas Daseeco, Jan 30 '17 at 17:15
@DouglasDaseeco Actually, it takes a minimum of two instructions to implement an `if(x==NULL)` construct, namely a compare followed by a conditional jump. (I don't think any architecture has a combined "jump if specific value"; it's always been "jump based on compare result".) Depending on architectural constraints it may also require one or more loads into CPU registers which in turn may push other things out of the CPU registers that will need to be reloaded later. But really, if your code is *that* time critical, you likely shouldn't be calling standard library functions in the first place. — user, Jan 31 '17 at 08:24

score 1 · Accepted Answer · answered Jan 24 '17 at 01:25

Dereferncing a null pointer can be a security risk on some architectures and operating systems, and whether it is a daemon or a handler or whether the function or method itself accesses the file system is not the sole determining factor in risk assessment. One cannot easily enumerate all exploits of indeterminate operations from source code alone.

In the code provided, the null is in the RHS (right hand side) of the assignment in the corresponding AST (abstract symbol tree) in all six cases. That means that security cannot be breached inside the function because of a null t.

There is a small risk, but it involves more than the mentioned dereference. It involves the subsequent use of the address space pointed to by the last two arguments. If invalid results placed into the target of those pointers could cause a security breach, then it is important to test t before dereferencing it.

score 1 · Answer 2 · answered Jan 24 '17 at 11:19

Not doing any checks whether a given pointer is NULL might not be exploitable at all, but I don't think in your case it matters if the program runs as daemon or not. The crucial question here is more whether the data which can lead to a null pointer dereference is user controlled or not.

In any case, you might want to fix this. It is never a good idea to leave a bug in the code which can crash a program once you have identified it and a fix is not expensive. The circumstances which now lead to the conclusion "not exploitable" might eventually change or the npd might otherwise lead to harm for your server. So if I were you I would simply add a if (t == NULL) in order to fix this.

Btw, if such a bug leads to code execution then it doesn't matter whether this happens in the daemon or in a subprocess. The only thing that matters is the OS user. Code execution is always bad, even if it is with a limited OS user.

`The crucial question here is more whether the data which can lead to a null pointer dereference is user controlled` yes it is. code execution is the kind of thing I want to know if it’s possible. — user2284570, Jan 27 '17 at 00:14

Is dereferencing a null pointer in C a security risk if the program isn’t a daemon, but a small script lauched as a separate process for each request?

2 Answers2