Skip to content
  • Jeff Layton's avatar
    cifs: fix potential buffer overrun in cifs.idmap handling code · 36960e44
    Jeff Layton authored
    
    
    The userspace cifs.idmap program generally works with the wbclient libs
    to generate binary SIDs in userspace. That program defines the struct
    that holds these values as having a max of 15 subauthorities. The kernel
    idmapping code however limits that value to 5.
    
    When the kernel copies those values around though, it doesn't sanity
    check the num_subauths value handed back from userspace or from the
    server. It's possible therefore for userspace to hand us back a bogus
    num_subauths value (or one that's valid, but greater than 5) that could
    cause the kernel to walk off the end of the cifs_sid->sub_auths array.
    
    Fix this by defining a new routine for copying sids and using that in
    all of the places that copy it. If we end up with a sid that's longer
    than expected then this approach will just lop off the "extra" subauths,
    but that's basically what the code does today already. Better approaches
    might be to fix this code to reject SIDs with >5 subauths, or fix it
    to handle the subauths array dynamically.
    
    At the same time, change the kernel to check the length of the data
    returned by userspace. If it's shorter than struct cifs_sid, reject it
    and return -EIO. If that happens we'll end up with fields that are
    basically uninitialized.
    
    Long term, it might make sense to redefine cifs_sid using a flexarray at
    the end, to allow for variable-length subauth lists, and teach the code
    to handle the case where the subauths array being passed in from
    userspace is shorter than 5 elements.
    
    Note too, that I don't consider this a security issue since you'd need
    a compromised cifs.idmap program. If you have that, you can do all sorts
    of nefarious stuff. Still, this is probably reasonable for stable.
    
    Cc: stable@kernel.org
    Reviewed-by: default avatarShirish Pargaonkar <shirishpargaonkar@gmail.com>
    Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
    36960e44