search

kravietz / pam_tacplus

TACACS+ protocol client library and PAM module in C. This PAM module support authentication, authorization (account management) and accounting (session management)performed using TACACS+ protocol designed by Cisco.
GNU Lesser General Public License v3.0
130 stars 97 forks source link

LeakSanitizer: detected memory leaks #172

Closed kravietz closed 2 years ago

kravietz commented 2 years ago

Found by ASAN

==10778==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 33 byte(s) in 2 object(s) allocated from:
    #0 0x7ff4a734e538 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x77538)
    #1 0x7ff4a70c0be1 in xstrdup libtac/lib/xalloc.c:62

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7ff4a73b5d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7ff4a70c0af0 in xcalloc libtac/lib/xalloc.c:30

Indirect leak of 13 byte(s) in 1 object(s) allocated from:
    #0 0x7ff4a73b5d28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7ff4a70c0af0 in xcalloc libtac/lib/xalloc.c:30

SUMMARY: AddressSanitizer: 70 byte(s) leaked in 4 allocation(s).
gollub commented 2 years ago

Are the backtrace truncated? What are the callers of those xalloc methods?

kravietz commented 2 years ago

@gollub Nope, that's what ASAN dumps :) These functions are called in plenty of places so I'm not surprised they aren't all freed perfectly...

kravietz commented 2 years ago

More details with extra ASAN options:

==14185==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f766db2dd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7f766d8382b0 in xcalloc libtac/lib/xalloc.c:31
    #2 0x7f766d83004e in _tac_add_attrib_pair libtac/lib/attrib.c:74
    #3 0x7f766d8335d3 in tac_author_read_timeout libtac/lib/author_r.c:263
    #4 0x565049269f66 in main /home/build/pam-tacplus/tacc.c:327
    #5 0x7f766c72bbf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)
    #6 0x56504926a5b9 in _start (/home/build/pam-tacplus/.libs/tacc+0x55b9)

Direct leak of 18 byte(s) in 1 object(s) allocated from:
    #0 0x7f766dac6538 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x77538)
    #1 0x7f766d8383a1 in xstrdup libtac/lib/xalloc.c:66
    #2 0x7f766d833f44 in tac_author_read_timeout libtac/lib/author_r.c:222
    #3 0x565049269f66 in main /home/build/pam-tacplus/tacc.c:327
    #4 0x7f766c72bbf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)
    #5 0x56504926a5b9 in _start (/home/build/pam-tacplus/.libs/tacc+0x55b9)

Direct leak of 15 byte(s) in 1 object(s) allocated from:
    #0 0x7f766dac6538 in strdup (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x77538)
    #1 0x7f766d8383a1 in xstrdup libtac/lib/xalloc.c:66
    #2 0x7f766d82e654 in tac_acct_read_timeout libtac/lib/acct_r.c:156
    #3 0x56504926a083 in main /home/build/pam-tacplus/tacc.c:377
    #4 0x7f766c72bbf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)
    #5 0x56504926a5b9 in _start (/home/build/pam-tacplus/.libs/tacc+0x55b9)

Indirect leak of 13 byte(s) in 1 object(s) allocated from:
    #0 0x7f766db2dd28 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xded28)
    #1 0x7f766d8382b0 in xcalloc libtac/lib/xalloc.c:31
    #2 0x7f766d82fe97 in _tac_add_attrib_pair libtac/lib/attrib.c:106
    #3 0x7f766d8335d3 in tac_author_read_timeout libtac/lib/author_r.c:263
    #4 0x565049269f66 in main /home/build/pam-tacplus/tacc.c:327
    #5 0x7f766c72bbf6 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21bf6)
    #6 0x56504926a5b9 in _start (/home/build/pam-tacplus/.libs/tacc+0x55b9)

SUMMARY: AddressSanitizer: 70 byte(s) leaked in 4 allocation(s).
kravietz commented 2 years ago

Note the leaks aren't really much of an actual problem because tacc is one-shot utility and memory is freed on exit(). Nonetheless, I want to clean them up for the sake of making the SAST tools happy :)

kravietz commented 2 years ago

OK, ultimately I've spotted all of them

https://github.com/kravietz/pam_tacplus/commit/0a28ffce33d8c0641b603666715e8cc776370426#diff-ddd7adfe8aa4940716e9ff6486889b61c05b61f1f0ef750636076bbc87b3db6eR378