mapper heap-buffer-overflow

[bowb]

When running against ASAN it detected a heap-buffer-overflow:

=================================================================                                                               
==78084==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60c000002ffd at pc 0x7fb185325118 bp 0x7ffcc0a9a850 sp 0x7ff
cc0a99ff8                                                                                                                       
READ of size 1 at 0x60c000002ffd thread T0                                                                                      
    #0 0x7fb185325117 in __interceptor_strchr ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:70
4                                                                                                                               
    #1 0x7fb182173810 in get_mapent /opt/pam_pkcs11/src/mappers/mapper.c:87                                                     
    #2 0x7fb182173b9c in mapfile_find /opt/pam_pkcs11/src/mappers/mapper.c:164                                                  
    #3 0x7fb1821728b2 in find_user /opt/pam_pkcs11/src/pam_pkcs11/mapper_mgr.c:270                                              
    #4 0x7fb1821714c4 in pam_sm_authenticate /opt/pam_pkcs11/src/pam_pkcs11/pam_pkcs11.c:578                                    
    #5 0x7fb18218f055  (/tmp/pam.T/lib/libpam.so.0+0x4055)
    #6 0x7fb18218e933 in pam_authenticate (/tmp/pam.T/lib/libpam.so.0+0x3933)
   ...

The issue is the get_mapent function in mapper.c isn't checking if the from variable is NULL or not when it is passed into strchr

[mskalski]

No -error is because from variable (initialized to mfile->pt) is not checked if it is properly \0-terminated.

It should be checked if read does not exceed buffer boundary using maybe memchr() function instead of strchr()?

[bowb]

I have also found memory leaks and I am working on cleaning them up. There is also an issue with the from++ reading an invalid byte in valgrind. I'm thinking about the following to fix both of these issues:

while(*from && isspace(*from)) {
   if(from - mfile->buffer +1 >= mfile->length) {
      return 0;
   }
   from++;
}