URL:
https://github.com/SSSD/sssd/pull/455
Title: #455: mmap_cache: make checks independent of input size
lslebodn commented:
"""
On (24/11/17 15:42), sumit-bose wrote:
I agree this is nitpicking and artificial, but I think strcmp() can
run outside the data table.
If the last slot in the data table is the last of an initgr record with many GIDs for a
user with a short user name, e.g. 'user' and the slot is fully used so that the
user name is at the end of the slot. data_len of in the initgr record (which is used for
strs_len in sss_mc_find_record() in this case) will be large keys much longer then
'user' will pass until the strcmp. If now due to a corruption the '0' at
the end of the user name (and the unique name) are replaced by a '-' with a key
like 'user-user-x' strcmp() will read the first byte of the free table when trying
to compare the 'x'. But I agree that even then it would not do any harm because we
are still reading from our own memory.
Ahh that's true because GIDs are stored before strings for initgtoup
and strs_len (sss_mc_get_strs_len) is actually data_len for initgroups.
That case could be covered by stricter check for length of input string
```
- if (key->len > strs_len) {
+ if (key->len + name_ptr > strs_len) {
/* The string cannot be in current record */
slot = sss_mc_next_slot_with_hash(rec, hash);
continue;
}
```
But such check should be probably after checking the value of `name_ptr`.
So probably inside "if" which would backup "corrupted" memory cache.
IMHO it's faster then trying to find '\0'.
LS
"""
See the full comment at
https://github.com/SSSD/sssd/pull/455#issuecomment-347185728