pkgtools/libnbcompat: incorrect sha256 hashes with -fstrict-aliasing

This issue was found in the Linux port of libnbcompat (see archiecobbs/libnbcompat#4 and the fix in archiecobbs/libnbcompat#7) but since the code in question is basically the same between the two codebases, I though it'd be prudent to open a bug report here as well.

The core issue is that the sha2.c code does some aliasing that GCC considers undefined behaviour when compiled with -fstrict-aliasing (this option is implied by -O2 which is enabled for most builds on most Linux distributions). However, it is only with GCC >= 11 that the code actually starts producing incorrect output. archiecobbs/libnbcompat#4 shows the difference in output for the standard NIST test vectors as well as my testing methodology, and archiecobbs/libnbcompat#3 is a PR against the port to add test programs to verify that the sha1, sha2 and ripemd160 code produces correct hashes for a set of test vectors. archiecobbs/libnbcompat#7 includes the fix for the port but this will need to be adjusted to match the NetBSD build system.

I'm not sure if any of the code I've written for the port would be welcome in NetBSD, I'm just leaving this comment to let you know that it's possible this might be an issue for NetBSD as well.

It's possible the mtree in src has the same issue because the sha2.c code in question is largely shared but -fno-strict-aliasing is used in some places around the codebase so I'm not sure if it ends up being set when compiling sha2.c. I'll boot up a NetBSD VM and take a look...

Seems that NetBSD 10 will use GCC 10.5 and NetBSD 9.3 has GCC 7.5, so the issue is not present in current NetBSD (some quick testing in a NetBSD 9.3 VM shows that mtree produces the correct output), but -fno-strict-aliasing should still be set to make sure it isn't silently missed when a newer GCC version is added.

@jsonn

cc @jsonn

archiecobbs/libnbcompat@864c1cf is a patch in the Linux port which fixes the strict aliasing issue.

diff --git a/sha2.c b/sha2.c
index bdcbd317bdd4..4c9436f98275 100644
--- a/sha2.c
+++ b/sha2.c
@@ -567,7 +567,7 @@ void SHA256_Final(sha2_byte digest[SHA256_DIGEST_LENGTH], SHA256_CTX* context) {
                        *context->buffer = 0x80;
                }
                /* Set the bit count: */
-               *(sha2_word64*)(void *)&context->buffer[SHA256_SHORT_BLOCK_LENGTH] = context->bitcount;
+               memcpy(&context->buffer[SHA256_SHORT_BLOCK_LENGTH], &context->bitcount, sizeof(context->bitcount));

                /* Final transform: */
                SHA256_Transform(context, (sha2_word32*)(void *)context->buffer);
@@ -870,8 +870,8 @@ static void SHA512_Last(SHA512_CTX* context) {
                *context->buffer = 0x80;
        }
        /* Store the length of input data (in bits): */
-       *(sha2_word64*)(void *)&context->buffer[SHA512_SHORT_BLOCK_LENGTH] = context->bitcount[1];
-       *(sha2_word64*)(void *)&context->buffer[SHA512_SHORT_BLOCK_LENGTH+8] = context->bitcount[0];
+       memcpy(&context->buffer[SHA512_SHORT_BLOCK_LENGTH], &context->bitcount[1], sizeof(*context->bitcount));
+       memcpy(&context->buffer[SHA512_SHORT_BLOCK_LENGTH+8], &context->bitcount[0], sizeof(*context->bitcount));

        /* Final transform: */
        SHA512_Transform(context, (sha2_word64*)(void *)context->buffer);

I'm pretty sure this was closed in error.

Stupid GitHub auto-close.

This looks akin to a fix already committed to our digest package:
b631703
In other words, quite reasonable for us to integrate.

chat/iamb: update to 0.0.8

devel/ruby-rgl: update to 0.6.6

tree-sitter-go: update to 0.20.0.

apache-ant: update to 1.10.14.

textproc/inlyne: update to 0.3.2

Update to version 1.9.27

www/hs-http2: update to http2-5.3.9

textproc/ruby-rails-dom-testing: update to 2.3.0

py-cmudict: update to 1.1.1

archivers/par2: update to 1.1.0

devel/serie: update to 0.6.0

devel/ruby-curses: update to 1.6.0

py-nh3: update to 0.3.5.

filesystems/fuse-bindfs: update to 1.18.4

p5-XML-LibXML: update to 2.0213.

(parallel/R-promises) Updated 1.3.2 to 1.5.0

(devel/R-lifecycle) Updated 1.0.4 to 1.0.5

p5-Crypt-OpenSSL-X509: update to 2.1.3.

net/ttl: update to 0.21.0

net/xfr: update to 0.9.21

Navigation Menu

Repository navigation

pkgtools/libnbcompat: incorrect sha256 hashes with -fstrict-aliasing #122

169 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

pkgtools/libnbcompat: incorrect sha256 hashes with -fstrict-aliasing #122

Description

Activity

cyphar commented on Jun 7, 2023

cyphar commented on Jun 7, 2023

bsiegert commented on Jun 7, 2023

cyphar commented on Jun 7, 2023

cyphar commented on Jun 12, 2023

jperkin commented on Jun 12, 2023

dhgutteridge commented on Jun 14, 2023

169 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions