Secretbox: explained non-portable behavior (#936)

Addresses #934

Some tools believe that comparing pointers, *even after converting them
to integers*, is undefined. A comment acknowledging this (as well as the
necessity of the comparison to begin with), can facilitate audits.

Co-authored-by: Frank Denis <124872+jedisct1@users.noreply.github.com>
This commit is contained in:
Loup Vaillant
2020-03-11 19:07:54 +01:00
committed by GitHub
co-authored by Frank Denis
parent 4bbc34c09c
commit e7e378fad1
2 changed files with 28 additions and 0 deletions
@@ -27,6 +27,13 @@ crypto_secretbox_detached(unsigned char *c, unsigned char *mac,
crypto_core_hsalsa20(subkey, n, k, NULL);
/* Allow the m and and c buffer to partially overlap, by calling
memmove() if necessary.
Note that there is no fully portable way to compare pointers.
Some tools even report undefined behavior, despite the conversion.
Nevertheless, this works on all supported platforms.
*/
if (((uintptr_t) c > (uintptr_t) m &&
(uintptr_t) c - (uintptr_t) m < mlen) ||
((uintptr_t) m > (uintptr_t) c &&
@@ -101,6 +108,13 @@ crypto_secretbox_open_detached(unsigned char *m, const unsigned char *c,
if (m == NULL) {
return 0;
}
/* Allow the m and and c buffer to partially overlap, by calling
memmove() if necessary.
Note that there is no fully portable way to compare pointers.
Some tools even report undefined behavior, despite the conversion.
Nevertheless, this works on all supported platforms.
*/
if (((uintptr_t) c > (uintptr_t) m &&
(uintptr_t) c - (uintptr_t) m < clen) ||
((uintptr_t) m > (uintptr_t) c &&
@@ -31,6 +31,13 @@ crypto_secretbox_xchacha20poly1305_detached(unsigned char *c,
crypto_core_hchacha20(subkey, n, k, NULL);
/* Allow the m and and c buffer to partially overlap, by calling
memmove() if necessary.
Note that there is no fully portable way to compare pointers.
Some tools even report undefined behavior, despite the conversion.
Nevertheless, this works on all supported platforms.
*/
if (((uintptr_t) c > (uintptr_t) m &&
(uintptr_t) c - (uintptr_t) m < mlen) ||
((uintptr_t) m > (uintptr_t) c &&
@@ -108,6 +115,13 @@ crypto_secretbox_xchacha20poly1305_open_detached(unsigned char *m,
if (m == NULL) {
return 0;
}
/* Allow the m and and c buffer to partially overlap, by calling
memmove() if necessary.
Note that there is no fully portable way to compare pointers.
Some tools even report undefined behavior, despite the conversion.
Nevertheless, this works on all supported platforms.
*/
if (((uintptr_t) c > (uintptr_t) m &&
(uintptr_t) c - (uintptr_t) m < clen) ||
((uintptr_t) m > (uintptr_t) c &&