src/sss_client/common.c: Use getpwnam_r to avoid clobbering struct passwd#8487
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the non-thread-safe getpwnam with its re-entrant version, getpwnam_r, to prevent potential data corruption when multiple threads are involved. The implementation follows the standard pattern for using getpwnam_r, including dynamic buffer allocation and resizing. However, I've found a potential memory leak in the error handling of realloc and provided a suggestion to fix it.
b66af65 to
c37669a
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly replaces the non-reentrant getpwnam with the reentrant getpwnam_r to prevent thread-safety issues. The implementation for handling ERANGE is mostly correct, but it has a potential denial-of-service vulnerability due to unbounded memory allocation. I've added a review comment with a suggested fix to address this by adding a size limit to the buffer.
|
Hi @salahcoronya, I think this reports a valid bug, thanks. But there is a number of (at least) stylistic issues (re SSSD coding style) with the patch. |
c37669a to
8084fb6
Compare
|
I went ahead and finalized it myself, follow the coding style document. |
894f67c to
6c85770
Compare
alexey-tikhonov
added
Changes requested
and removed
coverity
6c85770 to
bb74ab0
Compare
|
|
bb74ab0 to
55fb1fa
Compare
55fb1fa to
7b2ac0c
Compare
|
Thank you, I have no further comments. ACK. |
|
Note: Covscan is green. |
thalman
left a comment
thalman
left a comment
There was a problem hiding this comment.
Looks good to me, thanks
…sswd If something else uses PAM (like openrc, see OpenRC/openrc#984) and getpwnam, and calls something like pam_open_session, sssd's call to getpwnam in init_sssd_ids clobbers the cached value by the other program. Signed-off-by: Christopher Byrne <[email protected]> Reviewed-by: Alexey Tikhonov <[email protected]> Reviewed-by: Tomáš Halman <[email protected]>
7b2ac0c to
a2daebb
Compare
4 participants
If something else uses PAM (like openrc, see
OpenRC/openrc#984) and getpwnam, and calls something like pam_open_session, sssd's call to getpwnam in init_sssd_ids clobbers the cached value by the other program.