Re: [PATCH] crypto: dh - fix calculating encoded key size

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

 



On Wed, Jul 11, 2018 at 03:26:56PM +0800, Herbert Xu wrote:
> On Tue, Jul 10, 2018 at 08:59:05PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@xxxxxxxxxx>
> > 
> > It was forgotten to increase DH_KPP_SECRET_MIN_SIZE to include 'q_size',
> > causing an out-of-bounds write of 4 bytes in crypto_dh_encode_key(), and
> > an out-of-bounds read of 4 bytes in crypto_dh_decode_key().  Fix it.
> > Also add a BUG_ON() if crypto_dh_encode_key() doesn't exactly fill the
> > buffer, as that would have found this bug without resorting to KASAN.
> > 
> > Reported-by: syzbot+6d38d558c25b53b8f4ed@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: e3fe0ae12962 ("crypto: dh - add public key verification test")
> > Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> 
> Is it possible to return an error and use WARN_ON instead of BUG_ON?
> Or do the callers not bother to check for errors?
> 

The callers do check for errors, but at the point of the proposed BUG_ON() a
buffer overflow may have already occurred, so I think a BUG_ON() would be more
appropriate than a WARN_ON().  Of course, it would be better to prevent any
buffer overflow from occurring in the first place, but that's already the
purpose of the 'len != crypto_dh_key_len(params)' check; the issue was that
'crypto_dh_key_len()' calculated the wrong length.

- Eric



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]

  Powered by Linux