Bug 121773 - (PATCH)pam_smb bug cleanups
(PATCH)pam_smb bug cleanups
Product: Fedora
Classification: Fedora
Component: pam_smb (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Nalin Dahyabhai
Brian Brock
: Security
Depends On:
  Show dependency treegraph
Reported: 2004-04-27 14:07 EDT by Steve Grubb
Modified: 2007-11-30 17:10 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2004-07-27 17:13:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
Patch to cleanup bugs mentioned in this report (10.72 KB, patch)
2004-04-27 14:09 EDT, Steve Grubb
no flags Details | Diff

  None (edit)
Description Steve Grubb 2004-04-27 14:07:19 EDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.4.2)

Description of problem:
I performed a code review of this package and found a few issues that
needs addressing:

In pam_read_conf.c around line 19, fgets can return 0 on error. Not to
mention that the variables being passed have a width of 80 instead of
50. (See pam_smb_auth.c:136 for declarations.)

In pam_smb_auth.c around line 164, pam_get_user may return PAM_SUCCES,
but the variable name may be NULL or an empty string. A little farther
down, pam_get_item is not having its return code checked.

There are a couple of string arrays that should be labled const in
their header files.

In smbval/rfcnb-util.c, RFCNB_Free_Pkt does not return anything so it
was changed to a void function.

In smbval/session.c, there are 3 memory leaks in the error paths for
the variable con.

In mbval/smblib-util.c around line 357 is a fprintf to stderr. I
suspect that was supposed to be wrapped in #ifdef DEBUG. Who knows
what descriptor 2 points to at this point. It could cause damage
depending on what it is. Around line 446 is a series of equality
comparisons that are being bitwise or'ed. The compiler emits a warning
on this, making them || fixes the problem. Around 588, are 2 lines of
code with equality tests. The comment above leads me to believe that
assignment should have been occuring.

In support.c in the converse function are a couple of problems. pam
programming guidelines ask that authentication tokens are overwritten
before being free'd. _pam_drop_reply does this. It requires the "resp"
variable to be intact, though. I rearranged the code to handle this to
the pam guidelines. There was also a call to pam_set_item whose return
code is not being checked. If pam couldn't save it, the function
should fail to indicate this.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
Items were found in code review

Additional info:

I will attach a patch that addresses all these issues.
Comment 1 Steve Grubb 2004-04-27 14:09:29 EDT
Created attachment 99720 [details]
Patch to cleanup bugs mentioned in this report

Please apply before Fedora Core 2 final
Comment 2 Alan Cox 2004-07-27 17:13:38 EDT

Note You need to log in before you can comment on or make changes to this bug.