davidmoreno / onion

C library to create simple HTTP servers and Web Applications.
http://www.coralbits.com/libonion/
Other
2.01k stars 251 forks source link

Null pointer dereferences #300

Open Bogdanisar opened 2 years ago

Bogdanisar commented 2 years ago

Hi! I've run Cppcheck for static code analysis on the master branch of this project. I've found the following 3 code snippets which may contain either a redundant check or a NULL dereference:

1) At src/onion/onion.c:860:7:

      onion_listen_point *https = onion_https_new();
      https->server = onion;
      if (NULL == https) {
        ONION_ERROR
            ("Could not promote from HTTP to HTTPS. Certificate not set.");
      }
      https->port = port;
      https->hostname = hostname;

If the condition NULL == https can be true, then https->server is a NULL pointer dereference. Statement https->server = onion; was added in commit eaee81e03 , but before that commit the if condition was performed before the assignments. Maybe moving https->server = onion; after the condition is a good fix here?

2) At src/onion/sessions_redis.c:122:38:

  if (p == NULL) {
    redisReply *reply = redisCommand(p->context, "HDEL SESSIONS %b", session_id,
                                     strlen(session_id));
    ...
  } else {
    const char *json = onion_block_data(bl);
    redisReply *reply =
        redisCommand(p->context, "HSET SESSIONS %b %b", session_id,
                     strlen(session_id), json, strlen(json));
    ...
  }

Both code branches execute redisCommand(p->context, ...), but p is NULL for the first branch, so p->context will be a NULL pointer dereference.

3) At src/onion/poller.c:453:10:

  if (el && el->fd == fd) {
    ...
  }
  while (el->next) {
    ...
  }

If el can be NULL, then the statement while (el->next) will be a NULL pointer dereference. If not, the condition el && from if (el && el->fd == fd) is redundant.