URL:
https://github.com/SSSD/sssd/pull/455
Title: #455: mmap_cache: make checks independent of input size
lslebodn commented:
"""
On (24/11/17 07:10), sumit-bose wrote:
I kept the memchr() check mainly for the initgroups cache data. For
passwd and group the first element in the string/data area is the name and hence
'key->len > strs_len' makes sure the strcmp() will not read pass the current
object. For the initgroups data the name is 'somewhere' in the data area because
the data starts with a list of GIDs. So to avoid that strcmp() goes pass the end of the
data area with a long key in the case of a corruption the "length" of t_key has
to be determined. Using strnlen() would be possible as well but imo memchr() is more clear
here.
I'm sorry I cannot see any benefit for initgroups case.
I just can see performance impact in 99.9999% cases
with iterating over data twice (firstly with `memchr` secondly with `strcmp`)
It is not 100% cases because we still assume that memory cache can be
corrupted.
As I wrote in previous comment all checks before `memchr` are sufficient
to ensure that comparing data with NUL terminated input will not overflow area
of data table.
I'm fine with checking bounds because comparing numbers are fast. But I cannot
see any benefit of checking data twice.
Or did I miss something why directly using `strcmp` is unsafe?
LS
"""
See the full comment at
https://github.com/SSSD/sssd/pull/455#issuecomment-346824326