Fix UBSAN issue
Nothing with a real impact, just to silence sanitizers. From OSS-Fuzz: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /src/libfuzzer/FuzzerSHA1.cpp:132:14 in sec-tabselect.c:58:24: runtime error: negation of 1 cannot be represented in type 'mp_limb_t' (aka 'unsigned long') Patch attached. With Best Regards, Tim From f258d9cf51bca9a0c00caa478193e9ee4f7ebc5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= Date: Fri, 3 Nov 2017 17:04:43 +0100 Subject: [PATCH] sec-tabselect: Fix undefined behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes a UBSAN warning (fuzzing GnuTLS on OSS-Fuzz): negation of 1 cannot be represented in type 'mp_limb_t' (aka 'unsigned long') Signed-off-by: Tim Rühsen --- sec-tabselect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sec-tabselect.c b/sec-tabselect.c index e6bf2282..942c4247 100644 --- a/sec-tabselect.c +++ b/sec-tabselect.c @@ -55,7 +55,7 @@ sec_tabselect (mp_limb_t *rp, mp_size_t rn, mpn_zero (rp, rn); for (p = table; p < end; p += rn, k--) { - mp_limb_t mask = - (mp_limb_t) (k == 0); + mp_limb_t mask = (mp_limb_t) -(k == 0); for (i = 0; i < rn; i++) rp[i] += mask & p[i]; } -- 2.15.0.rc2 signature.asc Description: OpenPGP digital signature ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Fix UBSAN issue
Tim Rühsen writes: > Nothing with a real impact, just to silence sanitizers. Can you explain precisely what's undefined behavior in this code ? > diff --git a/sec-tabselect.c b/sec-tabselect.c > index e6bf2282..942c4247 100644 > --- a/sec-tabselect.c > +++ b/sec-tabselect.c > @@ -55,7 +55,7 @@ sec_tabselect (mp_limb_t *rp, mp_size_t rn, >mpn_zero (rp, rn); >for (p = table; p < end; p += rn, k--) > { > - mp_limb_t mask = - (mp_limb_t) (k == 0); As far as I understand, this should be perfectly portable C. (k == 0) evaluates to zero or one, with int type. This always fits in an mp_limb_t, hence (mp_limb_t) (k == 0) evaluates to zero or one, with mp_limb_t type. And since mp_limb_t is an *unsigned* type, arithmetic is always well defined as being performed modulo (ULONG_MAX + 1), including unary negation. So -(mp_limb_t) (k == 0) evaluates to zero or ULONG_MAX. (Assuming mp_limb_t is unsigned long, which it is an almost anything except 64-bit windows, where it's instead unsigned long long). But I may be missing something? These corners of the C language are a bit subtle. > + mp_limb_t mask = (mp_limb_t) -(k == 0); If the other way isn't broken, I'd prefer to change it like this. Because then one also has to think about why it produces the intended sign extension (which it does; it's not the same as (mp_limb_t) (unsigned) -(k == 0)). In general, both nettle and GMP depend on well-defined modulo arithmetic on unsigned types in *lots* of places. Any sanitizer which complains about that is pretty useless for this code. If your sanitizer complains by default, please use some option to disable that. And if there's no such option, please bug report the sanitizer tool. Best regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Fix UBSAN issue
On Sonntag, 5. November 2017 23:20:27 CET Niels Möller wrote: > Tim Rühsen writes: > > Nothing with a real impact, just to silence sanitizers. > > Can you explain precisely what's undefined behavior in this code ? > > > diff --git a/sec-tabselect.c b/sec-tabselect.c > > index e6bf2282..942c4247 100644 > > --- a/sec-tabselect.c > > +++ b/sec-tabselect.c > > @@ -55,7 +55,7 @@ sec_tabselect (mp_limb_t *rp, mp_size_t rn, > > > >mpn_zero (rp, rn); > >for (p = table; p < end; p += rn, k--) > > > > { > > > > - mp_limb_t mask = - (mp_limb_t) (k == 0); > > As far as I understand, this should be perfectly portable C. > > (k == 0) evaluates to zero or one, with int type. > > This always fits in an mp_limb_t, hence > > (mp_limb_t) (k == 0) evaluates to zero or one, with mp_limb_t type. > > And since mp_limb_t is an *unsigned* type, arithmetic is always well > defined as being performed modulo (ULONG_MAX + 1), including unary > negation. So > > -(mp_limb_t) (k == 0) evaluates to zero or ULONG_MAX. > > (Assuming mp_limb_t is unsigned long, which it is an almost anything > except 64-bit windows, where it's instead unsigned long long). > > But I may be missing something? These corners of the C language are a > bit subtle. > > > + mp_limb_t mask = (mp_limb_t) -(k == 0); > > If the other way isn't broken, I'd prefer to change it like this. > Because then one also has to think about why it produces the intended > sign extension (which it does; it's not the same as (mp_limb_t) > (unsigned) -(k == 0)). > > In general, both nettle and GMP depend on well-defined modulo arithmetic > on unsigned types in *lots* of places. Any sanitizer which complains > about that is pretty useless for this code. If your sanitizer complains > by default, please use some option to disable that. And if there's no > such option, please bug report the sanitizer tool. Thanks for taking such a detailed look ! Well, this is not really "my" sanitizer, it's "the" sanitizer (llvm/clang) used by Google's OSS-Fuzz. (gcc eventually implements clang's sanitizer features but is very behind). And of course you might be right and this is a bug in the sanitizer. I leave that discussion to the experts and open a bug at oss-fuzz as soon as I find the time. With Best Regards, Tim signature.asc Description: This is a digitally signed message part. ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Fix UBSAN issue
Tim Rühsen writes: > And of course you might be right and this is a bug in the sanitizer. > I leave that discussion to the experts and open a bug at oss-fuzz as soon as > I > find the time. Ok. Please add me to cc me on any related bugs, my personal address if that works, otherwise my work address ni...@google.com. Regards, /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677. Internet email is subject to wholesale government surveillance. ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Fix UBSAN issue
On Mittwoch, 8. November 2017 12:44:28 CET Niels Möller wrote: > Tim Rühsen writes: > > And of course you might be right and this is a bug in the sanitizer. > > I leave that discussion to the experts and open a bug at oss-fuzz as soon > > as I find the time. > > Ok. Please add me to cc me on any related bugs, my personal address if > that works, otherwise my work address ni...@google.com. Just opened an issue on Github. So i put a link here, if someone else is interested. https://github.com/google/oss-fuzz/issues/971 With Best Regards, Tim signature.asc Description: This is a digitally signed message part. ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
Re: Fix UBSAN issue
On Mittwoch, 8. November 2017 21:29:57 CET Tim Rühsen wrote: > On Mittwoch, 8. November 2017 12:44:28 CET Niels Möller wrote: > > Tim Rühsen writes: > > > And of course you might be right and this is a bug in the sanitizer. > > > I leave that discussion to the experts and open a bug at oss-fuzz as > > > soon > > > as I find the time. > > > > Ok. Please add me to cc me on any related bugs, my personal address if > > that works, otherwise my work address ni...@google.com. > > Just opened an issue on Github. So i put a link here, if someone else is > interested. > > https://github.com/google/oss-fuzz/issues/971 I just closed that bug since they agree with you that negating an unsigned type is defined. And the funny part is, those bug/issue reports (there were more on the oss- fuzz report) are gone now. Thanks for your investigation ! Regards, Tim signature.asc Description: This is a digitally signed message part. ___ nettle-bugs mailing list nettle-bugs@lists.lysator.liu.se http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs