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
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
Nikos Mavrogiannopoulos nmav@redhat.com writes:
The attached patches address the undefined behavior dependence for blowfish, twofish and des, and enable gitlab builds with asan and ubsan.
Thanks. I'm applying the last two patches (the des error was a little embarrassing...). For the first patch, I'd prefer to solve the problem slightly differently, see comments below.
@@ -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];
Use the READ_UINT32 macro instead, which includes the needed cast. And the same must be done for the blowfish_encrypt function just above, too.
--- 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])
I think it's simpler to change the return type of gf_multiply to uint32_t, and delete similar casts in the other places where it is used.
I'll try to take care of it, remind me if it isn't fixed within a few days.
Regards, /Niels
nettle-bugs@lists.lysator.liu.se
-
Nikos Mavrogiannopoulos -
Nikos Mavrogiannopoulos -
nisse＠lysator.liu.se