Project

General

Profile

Actions

Bug #14612

closed

idmap_cache_add_sid2pid() doesn't initialize winname_ttl, which can cause segfaults

Added by Matt Barden 5 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

In libidmap, the idmap cache '{gid/uid}2sid_winname' contains entries that can have either a SID or winname (or both). These entries also contain TTLs for the sid and winname, which are meant to be non-zero only when the corresponding item is non-NULL.
idmap_cache_lookup_winnameby{uid/gid}() looks up an entry in idmap_cache.uid2sid_winname and checks whether winname_ttl is greater than the current time before strdup()'ing winname. This usually works, because the idmap_cache_add_{sid/winname}2{uid/gid}() functions set one of these pairs to non-NULL/non-zero, and the other to NULL/zero.

However, the idmap_cache_add_sid2pid() function forgets to set winname_ttl to 0, even though it sets winname to NULL. This leaves winname_ttl uninitialized. If the program later calls idmap_getwinnamebypid() with IDMAP_REQ_FLG_USE_CACHE set in the flag argument, and if this uninitialized memory was set to a value greater than the time at which this function is called, idmap_cache_lookup_winnameby{uid/gid}() will strdup(NULL), causing a segfault.

The attached program can be built and used to reproduce this/test the fix. Simply run "./idmap-test S-1-5-32-544".

Testing was done with the reproduction program.


Files

Makefile (126 Bytes) Makefile Matt Barden, 2022-04-03 05:53 AM
idmap-test.c (2.58 KB) idmap-test.c Matt Barden, 2022-04-03 05:53 AM
Actions #1

Updated by Electric Monk 5 months ago

  • Gerrit CR set to 2093
Actions #2

Updated by Matt Barden 4 months ago

  • Subject changed from idmap_cache_add_sid2pid() doesn't initialize winname_ttl, which can cause segfaults to idmap_cache_add_sid2pid() doesn't initialize winname_ttl, which can cause segfaults
Actions #3

Updated by Matt Barden 4 months ago

  • Description updated (diff)
Actions #4

Updated by Electric Monk 4 months ago

  • Status changed from New to Closed

git commit c74f1323689f1dfea444e7fe0627eafc8c3b9076

commit  c74f1323689f1dfea444e7fe0627eafc8c3b9076
Author: Matt Barden <mbarden@tintri.com>
Date:   2022-04-18T16:55:38.000Z

    14612 idmap_cache_add_sid2pid() doesn't initialize winname_ttl, which can cause segfaults
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Jason King <jason.brian.king+illumos@gmail.com>
    Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF