NetBSD Problem Report #58039

From www@netbsd.org  Fri Mar 15 10:29:28 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 1B9291A924A
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 15 Mar 2024 10:29:28 +0000 (UTC)
Message-Id: <20240315102926.5FADA1A924B@mollari.NetBSD.org>
Date: Fri, 15 Mar 2024 10:29:26 +0000 (UTC)
From: netbsd-bugs@michael-kaufmann.ch
Reply-To: netbsd-bugs@michael-kaufmann.ch
To: gnats-bugs@NetBSD.org
Subject: Buffer overflow when writing a SHA512_224 or SHA512_256 digest
X-Send-Pr-Version: www-1.0

>Number:         58039
>Category:       lib
>Synopsis:       Buffer overflow when writing a SHA512_224 or SHA512_256 digest
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    riastradh
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 15 10:30:00 +0000 2024
>Closed-Date:    Mon Mar 25 17:56:17 +0000 2024
>Last-Modified:  Mon Mar 25 17:56:17 +0000 2024
>Originator:     Michael Kaufmann
>Release:        10.0 RC6
>Organization:
>Environment:
NetBSD netbsd.localdomain 10.0_RC6 NetBSD 10.0_RC6 (GENERIC) #0: Tue Mar 12 10:19:02 UTC 2024  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64
>Description:
When calculating digests with EVP_sha512_224() or EVP_sha512_256(), 64 bytes are written. Applications expect that 28 bytes are written (for EVP_sha512_224) or 32 bytes are written (for EVP_sha512_256). This results in a buffer overflow.

NetBSD uses a patched OpenSSL, with a custom implementation of the SHA512 functions. The original OpenSSL implementation does not have this problem.

Please also see problem report #51333

This bug has been discovered while debugging a curl problem: https://github.com/curl/curl/pull/13070
>How-To-Repeat:
Build and run this program. It crashes on NetBSD, runs fine on other operating systems, e.g. Linux.

gcc -Wall digest.c -lcrypto


#include <stdio.h>
#include <string.h>
#include <openssl/evp.h>

int main()
{
  const char* data = "1";
  size_t length = 1;

  unsigned char digest[32];
  int check_val = 123;

  printf("%d\n", check_val);

  memset(digest, '\0', sizeof(digest));

  EVP_MD_CTX* ctx = EVP_MD_CTX_create();
  if (!ctx) {
    fprintf(stderr, "EVP_MD_CTX_create() failed\n");
    return 1;
  }

  if (!EVP_DigestInit_ex(ctx, EVP_sha512_256(), NULL))
  {
    fprintf(stderr, "EVP_DigestInit_ex() failed\n");
    EVP_MD_CTX_destroy(ctx);
    return 1;
  }

  if (!EVP_DigestUpdate(ctx, data, length))
  {
    fprintf(stderr, "EVP_DigestUpdate() failed\n");
    EVP_MD_CTX_destroy(ctx);
    return 1;
  }

  unsigned int digest_size = 0;
  if (!EVP_DigestFinal_ex(ctx, digest, &digest_size))
  {
    fprintf(stderr, "EVP_DigestFinal_ex() failed\n");
    EVP_MD_CTX_destroy(ctx);
    return 1;
  }

  printf("digest size: %u\n", digest_size);
  printf("%d\n", check_val);

  EVP_MD_CTX_destroy(ctx);

  return 0;
}


Output on NetBSD:

123
digest size: 32
1964291709
[1]   Segmentation fault (core dumped) ./a.out


Output on Linux:

123
digest size: 32
123
>Fix:
Use an unpatched OpenSSL

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: lib-bug-people->riastradh
Responsible-Changed-By: riastradh@NetBSD.org
Responsible-Changed-When: Fri, 15 Mar 2024 14:34:06 +0000
Responsible-Changed-Why:
mine


From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58039 CVS commit: src
Date: Fri, 15 Mar 2024 15:32:08 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Mar 15 15:32:07 UTC 2024

 Modified Files:
 	src/distrib/sets/lists/debug: mi
 	src/distrib/sets/lists/tests: mi
 	src/tests/crypto/libcrypto: Makefile
 Added Files:
 	src/tests/crypto/libcrypto: t_sha512trunc.c

 Log Message:
 libcrypto: Add some trivial tests for truncated SHA-512 variants.

 These should use more of the test vectors from

 https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Secure-Hashing#Testing

 but this will do for now to detect the buffer overrun rake we left
 lying around for ourselves.

 PR lib/58039


 To generate a diff of this commit:
 cvs rdiff -u -r1.429 -r1.430 src/distrib/sets/lists/debug/mi
 cvs rdiff -u -r1.1310 -r1.1311 src/distrib/sets/lists/tests/mi
 cvs rdiff -u -r1.15 -r1.16 src/tests/crypto/libcrypto/Makefile
 cvs rdiff -u -r0 -r1.1 src/tests/crypto/libcrypto/t_sha512trunc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58039 CVS commit: src
Date: Fri, 15 Mar 2024 18:10:37 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Mar 15 18:10:37 UTC 2024

 Modified Files:
 	src/crypto/external/bsd/openssl/dist/crypto/evp: legacy_sha.c
 	src/crypto/external/bsd/openssl/dist/include/crypto: sha.h
 	src/crypto/external/bsd/openssl/dist/providers/implementations/digests:
 	    sha2_prov.c
 	src/crypto/external/bsd/openssl/lib/libcrypto: libc-sha2xx.c
 	src/tests/crypto/libcrypto: t_sha512trunc.c

 Log Message:
 libcrypto: Fix buffer overrun in truncated SHA-512 functions.

 Further fallout from the libc/openssl sha2 symbol collision.

 PR lib/58039


 To generate a diff of this commit:
 cvs rdiff -u -r1.1.1.2 -r1.2 \
     src/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c
 cvs rdiff -u -r1.1.1.2 -r1.2 \
     src/crypto/external/bsd/openssl/dist/include/crypto/sha.h
 cvs rdiff -u -r1.1.1.1 -r1.2 \
     src/crypto/external/bsd/openssl/dist/providers/implementations/digests/sha2_prov.c
 cvs rdiff -u -r1.3 -r1.4 \
     src/crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c
 cvs rdiff -u -r1.1 -r1.2 src/tests/crypto/libcrypto/t_sha512trunc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 15 Mar 2024 18:56:02 +0000
State-Changed-Why:
fixed in HEAD
needs pullup-10, pullup-9
inapplicable <9, openssl is too old in netbsd-8 to be affected


State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Fri, 15 Mar 2024 19:03:09 +0000
State-Changed-Why:
pullup-10 #637
pullup-9 #1819
inapplicable <9


From: Taylor R Campbell <riastradh@NetBSD.org>
To: Michael Kaufmann <netbsd-bugs@michael-kaufmann.ch>
Cc: gnats-bugs@NetBSD.org, netbsd-bugs@NetBSD.org
Subject: Re: lib/58039: Buffer overflow when writing a SHA512_224 or SHA512_256 digest
Date: Fri, 15 Mar 2024 19:57:30 +0000

 Thanks for the report!

 Obviously, the hole we've dug ourselves into with the OpenSSL SHA-2
 API is a bad state of affairs.

 Unfortunately, it is not as easy as simply not patching openssl.

 There is a namespace collision between NetBSD's libc and (unpatched)
 OpenSSL over names like `SHA512_Init' and `SHA512_Final', which are
 declared with different SHA512_CTX types of different sizes.

 And we have ABI compatibility requirements that prevent us from just
 dropping the symbols from libc, or, worse, pulling the definitions out
 of OpenSSL instead -- applications previously using the libc symbols
 and NetBSD's SHA512_CTX would suddenly get buffer overruns, because
 OpenSSL's SHA512_CTX is larger!

 Perhaps we could patch OpenSSL _just to rename the symbols_, and make
 sure OpenSSL's libcrypto is never using the libc symbols -- that way
 _new_ programs which include <openssl/sha.h> will get the OpenSSL
 symbols, and _old_ programs will still get the libc symbols, and
 OpenSSL will use its own internal API internally.  And maybe that will
 reduce our maintenance burden.

 But past attempts to dig out of this hole have met with various kinds
 of gnarly failure, and OpenSSL is ditching the easy-to-use C APIs for
 things like SHA-2 and AES anyway.  So for now I've just put another
 band-aid on the mess -- and added some automatic tests to catch the
 problem later.

From: Michael Kaufmann <netbsd-bugs@michael-kaufmann.ch>
To: Taylor R Campbell <riastradh@netbsd.org>
Cc: gnats-bugs@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: lib/58039: Buffer overflow when writing a SHA512_224 or
 SHA512_256 digest
Date: Fri, 15 Mar 2024 21:34:49 +0100

 First I thought that the NetBSD developers just prefer their own  
 implementation of SHA512 for some reason, but now I understand...  
 thank you for the explanation, and thank you for the bugfix! It's  
 really great to get a response and even a bugfix in less than a day :-)

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58039 CVS commit: [netbsd-10] src
Date: Mon, 25 Mar 2024 14:14:56 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Mar 25 14:14:56 UTC 2024

 Modified Files:
 	src/crypto/external/bsd/openssl/dist/crypto/evp [netbsd-10]:
 	    legacy_sha.c
 	src/crypto/external/bsd/openssl/dist/include/crypto [netbsd-10]: sha.h
 	src/crypto/external/bsd/openssl/dist/providers/implementations/digests [netbsd-10]:
 	    sha2_prov.c
 	src/crypto/external/bsd/openssl/lib/libcrypto [netbsd-10]:
 	    libc-sha2xx.c
 	src/distrib/sets/lists/debug [netbsd-10]: mi
 	src/distrib/sets/lists/tests [netbsd-10]: mi
 	src/tests/crypto/libcrypto [netbsd-10]: Makefile
 Added Files:
 	src/tests/crypto/libcrypto [netbsd-10]: t_sha512trunc.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #637):

 	crypto/external/bsd/openssl/dist/providers/implementations/digests/sha2_prov.c: revision 1.2
 	tests/crypto/libcrypto/t_sha512trunc.c: revision 1.1
 	tests/crypto/libcrypto/t_sha512trunc.c: revision 1.2
 	tests/crypto/libcrypto/Makefile: revision 1.16
 	distrib/sets/lists/tests/mi: revision 1.1311
 	crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c: revision 1.2
 	distrib/sets/lists/debug/mi: revision 1.430
 	crypto/external/bsd/openssl/dist/include/crypto/sha.h: revision 1.2
 	crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c: revision 1.4

 libcrypto: Add some trivial tests for truncated SHA-512 variants.
 These should use more of the test vectors from
 https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Secure-Hashing#Testing
 but this will do for now to detect the buffer overrun rake we left
 lying around for ourselves.
 PR lib/58039

 libcrypto: Fix buffer overrun in truncated SHA-512 functions.
 Further fallout from the libc/openssl sha2 symbol collision.
 PR lib/58039


 To generate a diff of this commit:
 cvs rdiff -u -r1.1.1.1.2.3 -r1.1.1.1.2.4 \
     src/crypto/external/bsd/openssl/dist/crypto/evp/legacy_sha.c
 cvs rdiff -u -r1.1.1.1.10.1 -r1.1.1.1.10.2 \
     src/crypto/external/bsd/openssl/dist/include/crypto/sha.h
 cvs rdiff -u -r1.1.1.1.2.2 -r1.1.1.1.2.3 \
     src/crypto/external/bsd/openssl/dist/providers/implementations/digests/sha2_prov.c
 cvs rdiff -u -r1.2.6.1 -r1.2.6.2 \
     src/crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c
 cvs rdiff -u -r1.394.2.5 -r1.394.2.6 src/distrib/sets/lists/debug/mi
 cvs rdiff -u -r1.1238.2.5 -r1.1238.2.6 src/distrib/sets/lists/tests/mi
 cvs rdiff -u -r1.14.10.1 -r1.14.10.2 src/tests/crypto/libcrypto/Makefile
 cvs rdiff -u -r0 -r1.2.2.2 src/tests/crypto/libcrypto/t_sha512trunc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58039 CVS commit: [netbsd-9] src
Date: Mon, 25 Mar 2024 14:26:16 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Mon Mar 25 14:26:16 UTC 2024

 Modified Files:
 	src/crypto/external/bsd/openssl/dist/crypto/evp [netbsd-9]: m_sha1.c
 	src/crypto/external/bsd/openssl/dist/include/crypto [netbsd-9]: sha.h
 	src/crypto/external/bsd/openssl/lib/libcrypto [netbsd-9]: libc-sha2xx.c
 	src/distrib/sets/lists/debug [netbsd-9]: mi
 	src/distrib/sets/lists/tests [netbsd-9]: mi
 	src/tests/crypto/libcrypto [netbsd-9]: Makefile
 Added Files:
 	src/tests/crypto/libcrypto [netbsd-9]: t_sha512trunc.c

 Log Message:
 Pull up following revision(s) (requested by riastradh in ticket #1819):

 	crypto/external/bsd/openssl/dist/providers/implementations/digests/sha2_prov.c: revision 1.2
 	  (applied to crypto/external/bsd/openssl/dist/crypto/evp/m_sha1.c)
 	tests/crypto/libcrypto/t_sha512trunc.c: revision 1.1
 	tests/crypto/libcrypto/t_sha512trunc.c: revision 1.2
 	tests/crypto/libcrypto/Makefile: revision 1.16
 	distrib/sets/lists/tests/mi: revision 1.1311
 	distrib/sets/lists/debug/mi: revision 1.430
 	crypto/external/bsd/openssl/dist/include/crypto/sha.h: revision 1.2
 	crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c: revision 1.4
 	(all via patch)

 libcrypto: Add some trivial tests for truncated SHA-512 variants.
 These should use more of the test vectors from
 https://csrc.nist.gov/Projects/Cryptographic-Algorithm-Validation-Program/Secure-Hashing#Testing
 but this will do for now to detect the buffer overrun rake we left
 lying around for ourselves.
 PR lib/58039

 libcrypto: Fix buffer overrun in truncated SHA-512 functions.
 Further fallout from the libc/openssl sha2 symbol collision.
 PR lib/58039


 To generate a diff of this commit:
 cvs rdiff -u -r1.11.2.1 -r1.11.2.2 \
     src/crypto/external/bsd/openssl/dist/crypto/evp/m_sha1.c
 cvs rdiff -u -r1.1.1.1.4.2 -r1.1.1.1.4.3 \
     src/crypto/external/bsd/openssl/dist/include/crypto/sha.h
 cvs rdiff -u -r1.1.6.1 -r1.1.6.2 \
     src/crypto/external/bsd/openssl/lib/libcrypto/libc-sha2xx.c
 cvs rdiff -u -r1.285.2.6 -r1.285.2.7 src/distrib/sets/lists/debug/mi
 cvs rdiff -u -r1.818.2.4 -r1.818.2.5 src/distrib/sets/lists/tests/mi
 cvs rdiff -u -r1.14 -r1.14.2.1 src/tests/crypto/libcrypto/Makefile
 cvs rdiff -u -r0 -r1.2.4.2 src/tests/crypto/libcrypto/t_sha512trunc.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 25 Mar 2024 17:56:17 +0000
State-Changed-Why:
fixed in head, pulled up to 10 and 9, inapplicable <9


>Unformatted:

Home	PR Database Search