The attached patches address the undefined behavior dependence
for blowfish, twofish and des, and enable gitlab builds with
asan and ubsan.

On Mon, 2016-02-29 at 13:29 +0100, Nikos Mavrogiannopoulos wrote:
> Hello,
>  I've now completed enabling the undefined sanitizer for gnutls, and
> may be a good idea to use it for nettle to.  The following patch
> enables running the test suite of nettle under libasan (to detect any
> invalid memory accesses/writes), and the undefined sanitizer.
> 
> I've run a test build, and the libasan build succeeds but the
> libubsan
> builds fail:
> https://gitlab.com/gnutls/nettle/builds/773956
> 
> Its complaints are not that critical for the targetted platforms but
> may be nice not to rely on undefined behavior.
> 
> regards,
> Nikos
From bfde220ad7709040f3fef4e51d86265eedfeba98 Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos <[email protected]>
Date: Thu, 10 Mar 2016 17:37:36 +0100
Subject: [PATCH 1/3] Enforce casting to unsigned type when needed to avoid
 undefined behavior

This corrects issues of the following type caught with -fsanitize=undefined
runtime error: left shift of 184 by 24 places cannot be represented in type 'int'
---
 blowfish.c | 4 ++--
 twofish.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/blowfish.c b/blowfish.c
index ba921e7..4d1b1ec 100644
--- a/blowfish.c
+++ b/blowfish.c
@@ -359,8 +359,8 @@ blowfish_decrypt (const struct blowfish_ctx *ctx,
     {
       uint32_t d1, d2;
 
-      d1 = src[0] << 24 | src[1] << 16 | src[2] << 8 | src[3];
-      d2 = src[4] << 24 | src[5] << 16 | src[6] << 8 | src[7];
+      d1 = (((uint32_t)src[0]) << 24) | src[1] << 16 | src[2] << 8 | src[3];
+      d2 = (((uint32_t)src[4]) << 24) | src[5] << 16 | src[6] << 8 | src[7];
       decrypt (ctx, &d1, &d2);
       dst[0] = (d1 >> 24) & 0xff;
       dst[1] = (d1 >> 16) & 0xff;
diff --git a/twofish.c b/twofish.c
index 45b0854..5bc375c 100644
--- a/twofish.c
+++ b/twofish.c
@@ -190,14 +190,14 @@ compute_s(uint32_t m1, uint32_t m2)
   uint32_t s = 0;
   int i;
   for (i = 0; i < 4; i++)
-    s |=  ((  gf_multiply(0x4D, m1,       rs_matrix[i][0])
+    s |= ((uint32_t)((  gf_multiply(0x4D, m1,       rs_matrix[i][0])
 	    ^ gf_multiply(0x4D, m1 >> 8,  rs_matrix[i][1])
 	    ^ gf_multiply(0x4D, m1 >> 16, rs_matrix[i][2])
 	    ^ gf_multiply(0x4D, m1 >> 24, rs_matrix[i][3])
 	    ^ gf_multiply(0x4D, m2,       rs_matrix[i][4])
 	    ^ gf_multiply(0x4D, m2 >> 8,  rs_matrix[i][5])
 	    ^ gf_multiply(0x4D, m2 >> 16, rs_matrix[i][6])
-	    ^ gf_multiply(0x4D, m2 >> 24, rs_matrix[i][7])) << (i*8));
+	    ^ gf_multiply(0x4D, m2 >> 24, rs_matrix[i][7]))) << (i*8));
   return s;
 }
 
-- 
2.5.0

From e107af6c768a6d45d74e34635c169175b068ab29 Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos <[email protected]>
Date: Thu, 10 Mar 2016 17:53:01 +0100
Subject: [PATCH 2/3] des: assign value after sanity check to avoid undefined
 behavior

This corrects issues of the following type caught with -fsanitize=undefined
des.c:176:42: runtime error: index 42 out of bounds for type 'int8_t [26][4]'
---
 des.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/des.c b/des.c
index f880f8f..ebde935 100644
--- a/des.c
+++ b/des.c
@@ -173,10 +173,13 @@ des_weak_p(const uint8_t *key)
   int8_t k1 = key[1] >> 1;
 
   unsigned hash = asso_values[k1 + 1] + asso_values[k0];
-  const int8_t *candidate = weak_key_hash[hash];
+  const int8_t *candidate;
 
   if (hash > 25)
     return 0;
+
+  candidate = weak_key_hash[hash];
+
   if (k0 != candidate[0]
       || k1 != candidate[1])
     return 0;
-- 
2.5.0

From d96d6757bccf064f07dbe6f6144c58ad4e5b206f Mon Sep 17 00:00:00 2001
From: Nikos Mavrogiannopoulos <[email protected]>
Date: Mon, 29 Feb 2016 13:08:36 +0100
Subject: [PATCH 3/3] .gitlab-ci.yml: enhance with builds using asan and ubsan

This allows running the test suite under address sanitizer and undefined
sanitizer.
---
 .gitlab-ci.yml | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 99b241e..8d44cbb 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -17,3 +17,19 @@ Build and Check (x86):
   - x86
   except:
   - tags
+Build and Check with ubsan:
+  script:
+  - ./.bootstrap && 
+    CXXFLAGS="-fsanitize=undefined -fno-sanitize-recover -g -O2" CFLAGS="-fsanitize=undefined -fno-sanitize-recover -g -O2" ./configure 
+    --disable-documentation && make -j4 && make check -j4
+  tags:
+  - ubsan
+  except:
+  - tags
+Build and Check with asan:
+  script:
+  - ./.bootstrap && 
+  - CXXFLAGS="-fsanitize=address -g -O2" CFLAGS="-fsanitize=address -g -O2" ./configure --disable-documentation &&
+    make -j4 && make check -j4
+  except:
+  - tags
-- 
2.5.0

_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Reply via email to