From 092f2ad0c84f5879f9f75cf4da2695e373b88d2d Mon Sep 17 00:00:00 2001
From: Olaf Schulz <olaf_schulz@posteo.de>
Date: Mon, 17 Oct 2022 20:31:52 +0200
Subject: [PATCH] Remove null pointer dereferences from #300

This commit resolves three null pointer dereferences mentioned by
Bogdanisar.

Comments about changes:

1) onion.c
 Add return statement if promotion to https had failed
 and set member variable only in !=NULL case.

2) Here, I'm unsure if my change is correct.

  I assume that the NULL-check is using the wrong variable!
  First of all, a check of the bl variable is missing. This indicates
  that (bl == NULL) would be the correct check.

  Moreover, the redis session probably should be deleted if the data
  dict is empty, too. Thus, I changed the line to
  if (onion_dict_count(data) == 0 || bl == NULL)

  Other variants would be
  if (bl == NULL)
  if (data == NULL || bl == NULL )

3) poller.c:
  If added a NULL check because poller->head is initialized
  with NULL in onion_poller_new.
---
 src/onion/onion.c          | 3 ++-
 src/onion/poller.c         | 7 ++++++-
 src/onion/sessions_redis.c | 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/onion/onion.c b/src/onion/onion.c
index 279fd188..420277ae 100644
--- a/src/onion/onion.c
+++ b/src/onion/onion.c
@@ -857,11 +857,12 @@ int onion_set_certificate_va(onion * onion, onion_ssl_certificate_type type,
           ? onion_low_strdup(first_listen_point->hostname) : NULL;
       onion_listen_point_free(first_listen_point);
       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.");
+        return -1;
       }
+      https->server = onion;
       https->port = port;
       https->hostname = hostname;
       onion->listen_points[0] = https;
diff --git a/src/onion/poller.c b/src/onion/poller.c
index 39d56784..e361c721 100644
--- a/src/onion/poller.c
+++ b/src/onion/poller.c
@@ -441,7 +441,12 @@ int onion_poller_remove(onion_poller * poller, int fd) {
   pthread_mutex_lock(&poller->mutex);
   ONION_DEBUG0("Trying to remove fd %d (%d)", fd, poller->n);
   onion_poller_slot *el = poller->head;
-  if (el && el->fd == fd) {
+  if (el == NULL){
+    ONION_WARNING("Poller slots empty. Can not remove fd %d", fd);
+    pthread_mutex_unlock(&poller->mutex);
+    return 0;
+  }
+  if (el->fd == fd) {
     ONION_DEBUG0("Removed from head %p", el);
 
     poller->head = el->next;
diff --git a/src/onion/sessions_redis.c b/src/onion/sessions_redis.c
index 6b2bf9b3..8c717dc2 100644
--- a/src/onion/sessions_redis.c
+++ b/src/onion/sessions_redis.c
@@ -118,7 +118,7 @@ void onion_sessions_redis_save(onion_sessions * sessions,
   pthread_mutex_lock(&p->mutex);
 #endif
 
-  if (p == NULL) {
+  if (onion_dict_count(data) == 0 || bl == NULL) {
     redisReply *reply = redisCommand(p->context, "HDEL SESSIONS %b", session_id,
                                      strlen(session_id));