[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

undefined sanitizer [was: fixes for running on centos7 under docker]


On Tue, 2018-04-17 at 18:02 +0200, Andreas Schneider wrote:
> On Friday, 13 April 2018 10:53:12 CEST Nikos Mavrogiannopoulos wrote:
> > Hi,
> 
> Hi Nikos,
> 
> >  These small fixes are needed for running the testsuite under
> > docker
> > and under centos7.
> 
> Thank you very much for your contributions. I've pushed it to master.

Thank you.

One more fix for an issue I saw when running with undefined sanitizer
is attached. That fixes undefined behavior, though in practice from
what I see it shouldn't cause any problems.

What I saw but didn't fix is an error when compiled with
-fsanitize=bounds-strict at this struct:

struct ssh_string_struct {
        uint32_t size;
        unsigned char data[1];
}

A way to work-around it (if you care) could be to modify it as
following:
struct ssh_string_struct {
        uint32_t size;
        unsigned char *data;
}

and allocate as (sizeof(struct ssh_string_struct)+data_size); then you
can point data to the end of the structure.

regards,
Nikos
From f0a3c8a69d72f2efc5f94d4c43760d7d798bbe4f Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos <nmav@xxxxxxxxxx>
Date: Wed, 18 Apr 2018 09:09:05 +0200
Subject: [PATCH 1/2] buffer: do not call explicit_bzero or memcpy with null
 arguments

This allows compiling and testing with undefined sanitizer.

Signed-off-by: Nikos Mavrogiannopoulos <nmav@xxxxxxxxxx>
---
 src/buffer.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/buffer.c b/src/buffer.c
index 28a345c6..ee3d34f6 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -144,10 +144,12 @@ static int realloc_buffer(struct ssh_buffer_struct *buffer, size_t needed) {
 	  new = malloc(needed);
 	  if (new == NULL) {
 		  return -1;
-      }
-	  memcpy(new, buffer->data,buffer->used);
-	  explicit_bzero(buffer->data, buffer->used);
-	  SAFE_FREE(buffer->data);
+          }
+	  if (buffer->used > 0) {
+		  memcpy(new, buffer->data, buffer->used);
+		  explicit_bzero(buffer->data, buffer->used);
+		  SAFE_FREE(buffer->data);
+	  }
   } else {
 	  new = realloc(buffer->data, needed);
 	  if (new == NULL) {
@@ -193,7 +195,8 @@ static void buffer_shift(ssh_buffer buffer){
 int ssh_buffer_reinit(struct ssh_buffer_struct *buffer)
 {
   buffer_verify(buffer);
-  explicit_bzero(buffer->data, buffer->used);
+  if (buffer->used > 0)
+    explicit_bzero(buffer->data, buffer->used);
   buffer->used = 0;
   buffer->pos = 0;
   if(buffer->allocated > 127) {
-- 
2.14.3


Follow-Ups:
Re: undefined sanitizer [was: fixes for running on centos7 under docker]Andreas Schneider <asn@xxxxxxxxxxxxxx>
References:
fixes for running on centos7 under dockerNikos Mavrogiannopoulos <nmav@xxxxxxxxxx>
Re: fixes for running on centos7 under dockerAndreas Schneider <asn@xxxxxxxxxxxxxx>
Archive administrator: postmaster@lists.cynapses.org