[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Security-discuss] handling integer overflows
From: |
Niels Möller |
Subject: |
Re: [Security-discuss] handling integer overflows |
Date: |
Sun, 01 Apr 2012 08:49:08 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (usg-unix-v) |
Nikos Mavrogiannopoulos <address@hidden> writes:
> I was wondering on how other gnu programs handle integer overflows.
I think in many cases it is simpler and good enough to enforce
reasonable (butsomewhat arbitrary) limits on the inputs.
E.g., if you require that all der length fields are less than 2^20,
that may be sufficient to avoid overflows (assuming that int is at
least 32 bits). Sure, it's perfectly ok with the spec to include a
multi-gigabyte cat movie in an x.509 certificate, but that doesn't mean
that it's a good idea to actually support such certificates.
> http://git.savannah.gnu.org/gitweb/?p=libtasn1.git;a=commitdiff;h=3873c6a49122e3f15901646e072938557acd3f8e
Some comments:
1. Do you really need signed values? For unsigned, addition overflow is
sligthly simpler,
s = x + y;
if (s < x) overflow...
Unsigned might also help a bit for the multiplication, see below.
2. Truncating the result to INT_MAX on overflow looks dangerous (but it
probably works fine, since you check for this case later and return
error).
3. For multiplication, looks like one factor is always a power of two,
so the division could be replaced by a shift (but it needs some
additional care to handle the negative case properly; the compiler
can't simply replace x / 256 by x >> 8 if x is signed, and I'm not
sure the C spec requires that shift of signed items is arithmetic
shift).
4. I think this type of code is prone to off-by-one-errors. I haven't
tried to check for that, but one has to consider that carefully, and
maybe some unit tests would make sense.
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.