Bug 121773 - (PATCH)pam_smb bug cleanups
Summary: (PATCH)pam_smb bug cleanups
Alias: None
Product: Fedora
Classification: Fedora
Component: pam_smb   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Nalin Dahyabhai
QA Contact: Brian Brock
Keywords: Security
Depends On:
TreeView+ depends on / blocked
Reported: 2004-04-27 18:07 UTC by Steve Grubb
Modified: 2007-11-30 22:10 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2004-07-27 21:13:38 UTC
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 18:09 UTC, Steve Grubb
no flags Details | Diff

Description Steve Grubb 2004-04-27 18:07:19 UTC
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 18:09:29 UTC
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 21:13:38 UTC

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