From 22d1fb47ac911700090346c445a1429ea02f8d34 Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Tue, 28 Dec 2021 14:25:19 +0100 Subject: [PATCH 01/12] Remove null pointer dereference This if is only true if both strchr return NULL, making sure that *q is a NULL dereference. The intention was to remove trailing '>' (or '\t') from the message-id (p) for add_news_message(). Adjust the condition. --- news.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news.c b/news.c index cd76c26..7791cde 100644 --- a/news.c +++ b/news.c @@ -443,7 +443,7 @@ loadNewsgroup0(ParsedURL *pu) continue; if (*p == '<') p++; - if (!(q = strchr(p, '>')) && !(q = strchr(p, '\t'))) + if ((q = strchr(p, '>')) || (q = strchr(p, '\t'))) *q = '\0'; if (!(s = checkHeader(buf, "Subject:"))) continue; From 6432709b8085c32e926f0dad3cd1d349f41ca715 Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Tue, 28 Dec 2021 16:13:01 +0100 Subject: [PATCH 02/12] Check for NULL before dereferencing a pointer get_auth_param() returns NULL in case of error. --- file.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/file.c b/file.c index ddc6214..964d1f4 100644 --- a/file.c +++ b/file.c @@ -1381,16 +1381,15 @@ AuthDigestCred(struct http_auth *ha, Str uname, Str pw, ParsedURL *pu, */ tmp = Strnew_m_charp("Digest username=\"", uname->ptr, "\"", NULL); - Strcat_m_charp(tmp, ", realm=", - get_auth_param(ha->param, "realm")->ptr, NULL); - Strcat_m_charp(tmp, ", nonce=", - get_auth_param(ha->param, "nonce")->ptr, NULL); + if ((s = get_auth_param(ha->param, "realm")) != NULL) + Strcat_m_charp(tmp, ", realm=", s->ptr, NULL); + if ((s = get_auth_param(ha->param, "nonce")) != NULL) + Strcat_m_charp(tmp, ", nonce=", s->ptr, NULL); Strcat_m_charp(tmp, ", uri=\"", uri->ptr, "\"", NULL); Strcat_m_charp(tmp, ", response=\"", rd->ptr, "\"", NULL); - if (algorithm) - Strcat_m_charp(tmp, ", algorithm=", - get_auth_param(ha->param, "algorithm")->ptr, NULL); + if (algorithm && (s = get_auth_param(ha->param, "algorithm"))) + Strcat_m_charp(tmp, ", algorithm=", s->ptr, NULL); if (cnonce) Strcat_m_charp(tmp, ", cnonce=\"", cnonce->ptr, "\"", NULL); From 1365cc1ecc838828f0291beb0c7fb03535518469 Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Tue, 28 Dec 2021 16:22:51 +0100 Subject: [PATCH 03/12] Check for NULL before dereferencing a pointer l is part of the exit condition of the while loop. If we exit the loop because l is NULL, we cannot dereference it. --- file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/file.c b/file.c index 964d1f4..a8f2fc6 100644 --- a/file.c +++ b/file.c @@ -8139,7 +8139,8 @@ getNextPage(Buffer *buf, int plen) l = l->next; } while (l && l->bpos); buf->firstLine = l; - buf->firstLine->prev = NULL; + if (l) + buf->firstLine->prev = NULL; } } pager_end: From 9f5c311e4561a855a1b6882c6003134fadab7d3b Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Tue, 28 Dec 2021 20:58:13 +0100 Subject: [PATCH 04/12] Do not call fclose() on a NULL pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The if clause is true if cache is NULL. man 3 fclose says: The behaviour of fclose() is undefined if the stream parameter is an illegal pointer, or is a descriptor already passed to a previous invo‐ cation of fclose(). Check if cache is NULL before calling fclose(). --- buffer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/buffer.c b/buffer.c index 083ecf6..f4a0377 100644 --- a/buffer.c +++ b/buffer.c @@ -707,7 +707,8 @@ readBufferCache(Buffer *buf) cache = fopen(buf->savecache, "r"); if (cache == NULL || fread1(clnum, cache) || fread1(tlnum, cache)) { - fclose(cache); + if (cache != NULL) + fclose(cache); buf->savecache = NULL; return -1; } From 1308f5efe56f05c12cde5b1f2c000f775b0bc8f5 Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Sun, 13 Feb 2022 10:49:23 +0100 Subject: [PATCH 05/12] Check for NULL before dereferencing the pointer In case of an error the whole frame is freed, break out of the loop to return early. --- w3mimg/fb/fb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/w3mimg/fb/fb.c b/w3mimg/fb/fb.c index 7960584..8abc1af 100644 --- a/w3mimg/fb/fb.c +++ b/w3mimg/fb/fb.c @@ -426,11 +426,13 @@ fb_frame_new(int w, int h, int n) for (i = 0; i < n; i++) { frame[i] = fb_image_new(w, h); + if (frame[i] == NULL) { + error = 1; + break; + } frame[i]->num = n; frame[i]->id = i; frame[i]->delay = 1000; - if (frame[i] == NULL) - error = 1; } if (error) { From d33a522936a97b92c21fc3010a068f481ecf28a3 Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Sun, 13 Feb 2022 10:51:06 +0100 Subject: [PATCH 06/12] Fix potential null pointer dereference --- w3mimg/fb/fb_imlib2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/w3mimg/fb/fb_imlib2.c b/w3mimg/fb/fb_imlib2.c index 1a5151c..477d385 100644 --- a/w3mimg/fb/fb_imlib2.c +++ b/w3mimg/fb/fb_imlib2.c @@ -84,6 +84,9 @@ draw(FB_IMAGE * img, Imlib_Image image) imlib_context_set_image(image); data = imlib_image_get_data_for_reading_only(); + if (data == NULL) + return; + for (j = 0; j < img->height; j++) { offset = img->width * j; for (i = 0; i < img->width; i++) { From 9eaf044c020af16f3af6f93060153654d3d65a2b Mon Sep 17 00:00:00 2001 From: Rene Kita Date: Fri, 15 Apr 2022 12:42:24 +0200 Subject: [PATCH 07/12] Check return value of Str... functions All these functions, StrmyUFgets, StrISgets, etc. can potentially return NULL. Add a check for it. --- file.c | 13 +++++++------ frame.c | 4 ++-- ftp.c | 6 ++++-- news.c | 8 +++++--- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/file.c b/file.c index a8f2fc6..53af4ac 100644 --- a/file.c +++ b/file.c @@ -608,7 +608,7 @@ readHeader(URLFile *uf, Buffer *newBuf, int thru, ParsedURL *pu) if (src) newBuf->header_source = tmpf; } - while ((tmp = StrmyUFgets(uf))->length) { + while ((tmp = StrmyUFgets(uf)) && tmp->length) { #ifdef USE_NNTP if (uf->scheme == SCM_NEWS && tmp->ptr[0] == '.') Strshrinkfirst(tmp, 1); @@ -6328,7 +6328,7 @@ file_feed() { Str s; s = StrISgets(_file_lp2); - if (s->length == 0) { + if (s && s->length == 0) { ISclose(_file_lp2); return NULL; } @@ -7339,7 +7339,7 @@ loadHTMLstream(URLFile *f, Buffer *newBuf, FILE * src, int internal) #endif if (IStype(f->stream) != IST_ENCODED) f->stream = newEncodedStream(f->stream, f->encoding); - while ((lineBuf2 = StrmyUFgets(f))->length) { + while ((lineBuf2 = StrmyUFgets(f)) && lineBuf2->length) { #ifdef USE_NNTP if (f->scheme == SCM_NEWS && lineBuf2->ptr[0] == '.') { Strshrinkfirst(lineBuf2, 1); @@ -7496,7 +7496,7 @@ loadGopherDir0(URLFile *uf, ParsedURL *pu) pre = 0; while (1) { - if (lbuf = StrUFgets(uf), lbuf->length == 0) + if (!(lbuf = StrUFgets(uf)) || lbuf->length == 0) break; if (lbuf->ptr[0] == '.' && (lbuf->ptr[1] == '\n' || lbuf->ptr[1] == '\r')) @@ -7666,7 +7666,7 @@ loadBuffer(URLFile *uf, Buffer *volatile newBuf) nlines = 0; if (IStype(uf->stream) != IST_ENCODED) uf->stream = newEncodedStream(uf->stream, uf->encoding); - while ((lineBuf2 = StrmyISgets(uf->stream))->length) { + while ((lineBuf2 = StrmyISgets(uf->stream)) && lineBuf2->length) { #ifdef USE_NNTP if (uf->scheme == SCM_NEWS && lineBuf2->ptr[0] == '.') { Strshrinkfirst(lineBuf2, 1); @@ -8090,7 +8090,8 @@ getNextPage(Buffer *buf, int plen) init_stream(&uf, SCM_UNKNOWN, NULL); for (i = 0; i < plen; i++) { - lineBuf2 = StrmyISgets(buf->pagerSource); + if (!(lineBuf2 = StrmyISgets(buf->pagerSource))) + return NULL; if (lineBuf2->length == 0) { /* Assume that `cmd == buf->filename' */ if (buf->filename) diff --git a/frame.c b/frame.c index c595c40..be1a961 100644 --- a/frame.c +++ b/frame.c @@ -532,7 +532,7 @@ createFrameFile(struct frameset *f, FILE * f1, Buffer *current, int level, !strcasecmp(frame.body->type, "text/plain")) { Str tmp; fprintf(f1, "
\n");
-		    while ((tmp = StrmyUFgets(&f2))->length) {
+		    while ((tmp = StrmyUFgets(&f2)) && tmp->length) {
 			tmp = convertLine(NULL, tmp, HTML_MODE, &charset,
 					  doc_charset);
 			fprintf(f1, "%s", html_quote(tmp->ptr));
@@ -549,7 +549,7 @@ createFrameFile(struct frameset *f, FILE * f1, Buffer *current, int level,
 		    do {
 			if (*p == '\0') {
 			    Str tmp = StrmyUFgets(&f2);
-			    if (tmp->length == 0)
+			    if (!tmp || tmp->length == 0)
 				break;
 			    tmp = convertLine(NULL, tmp, HTML_MODE, &charset,
 					      doc_charset);
diff --git a/ftp.c b/ftp.c
index 2ca0247..f73d239 100644
--- a/ftp.c
+++ b/ftp.c
@@ -70,7 +70,8 @@ ftp_command(FTP ftp, char *cmd, char *arg, int *status)
     if (!status)
 	return NULL;
     *status = -1;		/* error */
-    tmp = StrISgets(ftp->rf);
+    if (!(tmp = StrISgets(ftp->rf)))
+	return NULL;
     if (IS_DIGIT(tmp->ptr[0]) && IS_DIGIT(tmp->ptr[1]) &&
 	IS_DIGIT(tmp->ptr[2]) && tmp->ptr[3] == ' ')
 	sscanf(tmp->ptr, "%d", status);
@@ -87,7 +88,8 @@ ftp_command(FTP ftp, char *cmd, char *arg, int *status)
      * with the same code, followed immediately by Space , 
      * optionally some text, and the Telnet end-of-line code. */
     while (1) {
-	tmp = StrISgets(ftp->rf);
+	if (!(tmp = StrISgets(ftp->rf)))
+	    break;
 	if (IS_DIGIT(tmp->ptr[0]) && IS_DIGIT(tmp->ptr[1]) &&
 	    IS_DIGIT(tmp->ptr[2]) && tmp->ptr[3] == ' ') {
 	    sscanf(tmp->ptr, "%d", status);
diff --git a/news.c b/news.c
index 7791cde..66667e4 100644
--- a/news.c
+++ b/news.c
@@ -51,7 +51,7 @@ news_command(News * news, char *cmd, char *arg, int *status)
 	return NULL;
     *status = -1;
     tmp = StrISgets(news->rf);
-    if (tmp->length)
+    if (tmp && tmp->length)
 	sscanf(tmp->ptr, "%d", status);
     return tmp;
 }
@@ -396,7 +396,8 @@ loadNewsgroup0(ParsedURL *pu)
     if (status == 224) {
 	f.scheme = SCM_NEWS;
 	while (1) {
-	    tmp = StrISgets(current_news.rf);
+	    if (!(tmp = StrISgets(current_news.rf)))
+		break;
 	    if (NEWS_ENDLINE(tmp->ptr))
 		break;
 	    if (sscanf(tmp->ptr, "%d", &i) != 1)
@@ -474,7 +475,8 @@ loadNewsgroup0(ParsedURL *pu)
     if (status != 215)
 	goto news_end;
     while (1) {
-	tmp = StrISgets(current_news.rf);
+	if (!(tmp = StrISgets(current_news.rf)))
+	    break;
 	if (NEWS_ENDLINE(tmp->ptr))
 	    break;
 	if (flag < 2) {

From a0b3c1b46762cb7f12aececd0e29838eedabe872 Mon Sep 17 00:00:00 2001
From: Rene Kita 
Date: Fri, 15 Apr 2022 12:43:36 +0200
Subject: [PATCH 08/12] Fix potential null dereference

---
 etc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/etc.c b/etc.c
index d50adad..805bfa0 100644
--- a/etc.c
+++ b/etc.c
@@ -1596,7 +1596,8 @@ file_to_url(char *file)
     char *host = NULL;
 #endif
 
-    file = expandPath(file);
+    if (!(file = expandPath(file)))
+	return NULL;
 #ifdef SUPPORT_NETBIOS_SHARE
     if (file[0] == '/' && file[1] == '/') {
 	char *p;

From 80642d1fe7ec6343825ef046408a82e45a5cdfcf Mon Sep 17 00:00:00 2001
From: Rene Kita 
Date: Fri, 15 Apr 2022 12:46:08 +0200
Subject: [PATCH 09/12] Fix potential null dereference

---
 file.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/file.c b/file.c
index 53af4ac..bd9d0c2 100644
--- a/file.c
+++ b/file.c
@@ -3103,11 +3103,14 @@ purgeline(struct html_feed_environ *h_env)
 {
     char *p, *q;
     Str tmp;
+    TextLine *tl;
 
     if (h_env->buf == NULL || h_env->blank_lines == 0)
 	return;
 
-    p = rpopTextLine(h_env->buf)->line->ptr;
+    if (!(tl = rpopTextLine(h_env->buf)))
+	return;
+    p = tl->line->ptr;
     tmp = Strnew();
     while (*p) {
 	q = p;

From e6998fc9a46f0912272f88affa9b97fe5f61cbfc Mon Sep 17 00:00:00 2001
From: Rene Kita 
Date: Fri, 15 Apr 2022 12:46:36 +0200
Subject: [PATCH 10/12] Fix potential null dereference

---
 main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/main.c b/main.c
index 0082068..3f39ec9 100644
--- a/main.c
+++ b/main.c
@@ -6481,8 +6481,8 @@ followTab(TabBuffer * tab)
 	Buffer *c, *p;
 
 	c = Currentbuf;
-	p = prevBuffer(c, buf);
-	p->nextBuffer = NULL;
+	if ((p = prevBuffer(c, buf)))
+	    p->nextBuffer = NULL;
 	Firstbuf = buf;
 	deleteTab(CurrentTab);
 	CurrentTab = tab;
@@ -6522,8 +6522,8 @@ tabURL0(TabBuffer * tab, char *prompt, int relative)
 	Buffer *c, *p;
 
 	c = Currentbuf;
-	p = prevBuffer(c, buf);
-	p->nextBuffer = NULL;
+	if ((p = prevBuffer(c, buf)))
+	    p->nextBuffer = NULL;
 	Firstbuf = buf;
 	deleteTab(CurrentTab);
 	CurrentTab = tab;

From e8a41ecfccbd276be05dc9975283edc1bcf417b9 Mon Sep 17 00:00:00 2001
From: Rene Kita 
Date: Sun, 17 Apr 2022 16:31:39 +0200
Subject: [PATCH 11/12] Exit if we cannot allocate a new tab during start

---
 main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/main.c b/main.c
index 3f39ec9..005d9b2 100644
--- a/main.c
+++ b/main.c
@@ -1079,6 +1079,10 @@ main(int argc, char **argv)
 	    newbuf->search_header = search_header;
 	if (CurrentTab == NULL) {
 	    FirstTab = LastTab = CurrentTab = newTab();
+	    if (!FirstTab) {
+		fprintf(stderr, "%s\n","Can't allocated memory");
+		exit(1);
+	    }
 	    nTab = 1;
 	    Firstbuf = Currentbuf = newbuf;
 	}

From a3b9887113ef44ac9fd2baa5dae28428ee8649e5 Mon Sep 17 00:00:00 2001
From: Rene Kita 
Date: Fri, 15 Apr 2022 13:11:21 +0200
Subject: [PATCH 12/12] Enable -Wnull-dereference by default

---
 Makefile.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.in b/Makefile.in
index 42027d6..7241e0c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -35,7 +35,8 @@ RC_DIR = @RC_DIR@
 ETC_DIR = $(sysconfdir)
 CONF_DIR = $(sysconfdir)/$(PACKAGE)
 
-CFLAGS = -Wall -I. -I$(top_srcdir) @CFLAGS@ $(CPPFLAGS) $(DEFS) $(OPTS)
+WARNINGS= -Wall -Wnull-dereference
+CFLAGS = $(WARNINGS) -I. -I$(top_srcdir) @CFLAGS@ $(CPPFLAGS) $(DEFS) $(OPTS)
 WCCFLAGS = @WCCFLAGS@
 CPPFLAGS = @CPPFLAGS@
 DEFS = @DEFS@ -DAUXBIN_DIR=\"$(AUXBIN_DIR)\" \