4

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 ?

9
  • 1
    What I would like to know is if it is definitely not a vulnerability. Please note the process don’t create any file. Jun 28 '16 at 14:20
  • 1
    Do errors take time? Does it dump core and take up disk space? 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). Jun 28 '16 at 15:58
  • 1
    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
  • 1
    @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? :) Jan 30 '17 at 17:15
1

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.

1

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.

1
  • 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. Jan 27 '17 at 0:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.