View Issue Details

IDProjectCategoryView StatusLast Update
0007620GNUnetutil librarypublic2024-02-29 22:46
Reporterulfvonbelow Assigned Toschanzen  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product VersionGit master 
Target Version0.19.4Fixed in Version0.19.4 
Summary0007620: 1-byte over-accesses in util
DescriptionIn GNUNET_CRYPTO_crc16_step, we read a short where there's only a byte left.

In GNUNET_CRYPTO_hash_from_string2, upper_enc isn't large enough to include the null terminator, so that gets written to the first byte of whatever happens to be next.
Steps To Reproduce./configure --enable-sanitizer
make
make install
make check
Additional InformationPatch attached.
Tagspatch
Attached Files
0001-UTIL-fix-one-byte-buffer-over-reads.patch (1,744 bytes)   
From 17a3f72e852cb2c804eac64040d6bef3b2f8d40e Mon Sep 17 00:00:00 2001
From: ulfvonbelow <strilen@tilde.club>
Date: Sun, 29 Jan 2023 05:15:30 -0600
Subject: [PATCH] UTIL: fix one-byte buffer over-reads.

GNUNET_CRYPTO_hash_from_string2 uses enclen as the length of its buffer that
it passes to GNUNET_STRINGS_utf8_toupper, but GNUNET_STRINGS_utf8_toupper adds
a null terminator, so it should be enclen+1.

GNUNET_CRYPTO_crc16_step reads 1 byte past the end of the buffer passed to
it. It masks out that byte in computing the result, but it's still technically
an overread and could in extremely-rare circumstances cause a segmentation or
access fault. It also upsets sanitizers, preventing other bugs from being found.
---
 src/util/crypto_crc.c  | 2 +-
 src/util/crypto_hash.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/crypto_crc.c b/src/util/crypto_crc.c
index 9328f2b84..f93b5b0b3 100644
--- a/src/util/crypto_crc.c
+++ b/src/util/crypto_crc.c
@@ -114,7 +114,7 @@ GNUNET_CRYPTO_crc16_step (uint32_t sum, const void *buf, size_t len)
   for (; len >= 2; len -= 2)
     sum += *(hdr++);
   if (len == 1)
-    sum += (*hdr) & ntohs (0xFF00);
+    sum += ntohs(*((uint8_t *)hdr) << 8);
   return sum;
 }
 
diff --git a/src/util/crypto_hash.c b/src/util/crypto_hash.c
index e45cb42e0..95c5c3480 100644
--- a/src/util/crypto_hash.c
+++ b/src/util/crypto_hash.c
@@ -73,7 +73,7 @@ GNUNET_CRYPTO_hash_from_string2 (const char *enc,
                                  size_t enclen,
                                  struct GNUNET_HashCode *result)
 {
-  char upper_enc[enclen];
+  char upper_enc[enclen+1];
   char *up_ptr = upper_enc;
 
   if (GNUNET_OK != GNUNET_STRINGS_utf8_toupper (enc, up_ptr))
-- 
2.38.1

Activities

schanzen

2023-02-06 05:31

administrator   ~0019759

Thank you!
3b5473735cb495ca50139adeb27e5135accaa22d

schanzen

2023-06-01 20:26

administrator   ~0020245

released some time ago

Issue History

Date Modified Username Field Change
2023-01-29 21:17 ulfvonbelow New Issue
2023-01-29 21:17 ulfvonbelow Tag Attached: bug
2023-01-29 21:17 ulfvonbelow Tag Attached: patch
2023-01-29 21:17 ulfvonbelow File Added: 0001-UTIL-fix-one-byte-buffer-over-reads.patch
2023-02-06 05:31 schanzen Assigned To => schanzen
2023-02-06 05:31 schanzen Status new => resolved
2023-02-06 05:31 schanzen Resolution open => fixed
2023-02-06 05:31 schanzen Fixed in Version => 0.19.4
2023-02-06 05:31 schanzen Note Added: 0019759
2023-02-06 05:46 schanzen Target Version => 0.19.4
2023-06-01 20:26 schanzen Note Added: 0020245
2023-06-01 20:26 schanzen Status resolved => closed
2024-02-29 22:46 Christian Grothoff Tag Detached: bug