resurrecting-open-source-projects / dcfldd

Enhanced version of dd for forensics and security
GNU General Public License v2.0
90 stars 19 forks source link

sha2: fix aliasing violation #24

Closed thesamesam closed 3 months ago

thesamesam commented 4 months ago

sha2: fix aliasing violation

&context->buffer is uint8_t*, but we try to access it as sha2_word64*, which is an aliasing violation (undefined behaviour).

Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. This is just as fast with any modern compiler.

Bug: https://gcc.gnu.org/PR114698 Bug: https://github.com/NetBSD/pkgsrc/issues/122 Bug: https://github.com/archiecobbs/libnbcompat/issues/4 Bug: https://bugs.launchpad.net/ubuntu-power-systems/+bug/2033405 Signed-off-by: Sam James sam@gentoo.org

thesamesam commented 4 months ago

I've used the same patch from the linked bugs but it's also the obvious patch for the problem.

peter-bergner commented 4 months ago

See also Canonical Bug: #2033405

thesamesam commented 3 months ago

If this is a theoretical or a practical problem at runtime also?

Please see https://gcc.gnu.org/PR114698 where this actually did break dcfldd itself, but also https://github.com/NetBSD/pkgsrc/issues/122 and https://github.com/archiecobbs/libnbcompat/issues/4 where the same file is used in other projects and it led to wrong results.

I did mention that in the commit message but maybe it wasn't explicit.

How to invoke GCC to make it detect this issue — I tried and it kept silent.

Unfortunately, GCC's strict aliasing warnings are known to be deficient and easy to fool. See https://gcc.gnu.org/PR113151. You can sometimes get GCC to warn on things by using -Wstrict-aliasing=2 but then it has more risk of FPs and it still is easy to fool.

davidpolverari commented 3 months ago

Hi, @hartwork!

What I do not yet understand is:

* If this is a theoretical or a practical problem at runtime also?

* How to invoke GCC to make it detect this issue — I tried and it kept silent.

I also tried to reproduce the issue presented on https://gcc.gnu.org/PR114698 by using a Debian Sid ppc64el image emulated under QEMU. I had no wrong results after building dcfldd using GCC 13 with -O3. I will have another (and closer) look at it as soon as I have time.

Regards, David

davidpolverari commented 3 months ago

... I had no wrong results after building dcfldd using GCC 13 with -O3. I will have another (and closer) look at it as soon as I have time.

Well, I must had done something wrong last time I tested, because I had tests failing now after building with -O3 on ppc64el with GCC 13. Looking at it now.

davidpolverari commented 3 months ago

Thanks for your help!

thesamesam commented 3 months ago

Thank you!

hartwork commented 3 months ago

@davidpolverari is there a chance to release 1.9.2 with this critical patch? Without a new release, all downstreams will have to bundle the patch downstream and become aware some way also. A new release would help awareness while reducing multiplication of work. What do you think?