tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

out of bounds read in humanize_number()



I've made the following commit to FreeBSD's humanize_number which has
diverged quite a lot from NetBSD's, but it looks like we've shared this
bug since 2004 or so.

The error occurs due to this loop:

for (i = 0; bytes >= max - 50 && i < maxscale; i++)

'i' then gets used as a scale passed to SCALE2PREFIX(), but maxscale
would be more correctly named numscale so if i == 7 it's out of bounds.

-- Brooks

P.S. IMO the real cause of this off-by-one error is the failure to use a
proper array of prefixes.  FreeBSD makes it worse by adding Ki, Mi, etc
so the prefixes strings are all three characters per suffix making the
code unreadable.

----- Forwarded message from Brooks Davis <brooks%FreeBSD.org@localhost> -----

Date: Thu, 13 Apr 2017 15:49:33 +0000 (UTC)
From: Brooks Davis <brooks%FreeBSD.org@localhost>
To: src-committers%freebsd.org@localhost, svn-src-all%freebsd.org@localhost,
	svn-src-head%freebsd.org@localhost
Subject: svn commit: r316766 - head/lib/libutil

Author: brooks
Date: Thu Apr 13 15:49:32 2017
New Revision: 316766
URL: https://svnweb.freebsd.org/changeset/base/316766

Log:
  Correct an out of bounds read with HN_AUTOSCALE and very large numbers.
  
  The maximum scale is 6 (K, M, G, T, P, E) (B is 0).
  
  Overly large explict scales were checked correctly, but for sufficently
  large numbers HN_AUTOSCALE would get to 7 resulting in an out of bounds
  read.
  
  Found with humanize_number_test and CHERI bounds checking.
  
  Reviewed by:	emaste
  Obtained from:	CheriBSD
  MFC after:	1 week
  Sponsored by:	DARPA, AFRL
  Differential Revision:	https://reviews.freebsd.org/D10376

Modified:
  head/lib/libutil/humanize_number.c

Modified: head/lib/libutil/humanize_number.c
==============================================================================
--- head/lib/libutil/humanize_number.c	Thu Apr 13 15:47:58 2017	(r316765)
+++ head/lib/libutil/humanize_number.c	Thu Apr 13 15:49:32 2017	(r316766)
@@ -43,7 +43,7 @@ __FBSDID("$FreeBSD$");
 #include <locale.h>
 #include <libutil.h>
 
-static const int maxscale = 7;
+static const int maxscale = 6;
 
 int
 humanize_number(char *buf, size_t len, int64_t quotient,
@@ -64,7 +64,7 @@ humanize_number(char *buf, size_t len, i
 		return (-1);
 	if (scale < 0)
 		return (-1);
-	else if (scale >= maxscale &&
+	else if (scale > maxscale &&
 	    ((scale & ~(HN_AUTOSCALE|HN_GETSCALE)) != 0))
 		return (-1);
 	if ((flags & HN_DIVISOR_1000) && (flags & HN_IEC_PREFIXES))


----- End forwarded message -----

Attachment: signature.asc
Description: PGP signature



Home | Main Index | Thread Index | Old Index