Bug 120168 - CAN-2004-2392 libuser serious programming bugs
CAN-2004-2392 libuser serious programming bugs
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: libuser (Show other bugs)
rawhide
All Linux
medium Severity high
: ---
: ---
Assigned To: Miloslav Trmač
:
Depends On:
Blocks: FC2Blocker FC3Target
  Show dependency treegraph
 
Reported: 2004-04-06 12:08 EDT by Steve Grubb
Modified: 2007-11-30 17:10 EST (History)
4 users (show)

See Also:
Fixed In Version: 0.51.10-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2004-09-02 18:22:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to fix all the problems listed above. (7.21 KB, patch)
2004-04-06 12:10 EDT, Steve Grubb
no flags Details | Diff
Current patch I'm testing (8.12 KB, patch)
2004-07-27 15:08 EDT, Steve Grubb
no flags Details | Diff

  None (edit)
Description Steve Grubb 2004-04-06 12:08:48 EDT
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.4.2)
Gecko/20040308

Description of problem:
During code review I could a number of serious programming bugs:

If there was a failure reading config data, the memory buffer was
taken to be valid.

On a read failure of /dev/urandom, buffer was taken to be valid. The
function could not indicate that a failure occurred or even retry if
errno == EINTR.

Upon a read failure, salt may be mostly 0's.

During unlock, a structure was being passed by value to a variadic
function.

length returned by read was being caught in an unsigned variable which
would later cause write to output 4294967295 bytes of data.

flcose was being passed a NULL pointer in many places.



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

How reproducible:
Always

Steps to Reproduce:
Found during code review
    

Additional info:

I will attach a patch that fixes all of these issues. I did not review
prior versions to see if they are susceptible to all these problems.
Comment 1 Steve Grubb 2004-04-06 12:10:43 EDT
Created attachment 99149 [details]
Patch to fix all the problems listed above.

Please apply before fedora core 2 final.
Comment 2 Warren Togami 2004-04-10 23:59:26 EDT
This needs code review.
Comment 3 Nalin Dahyabhai 2004-04-12 23:32:28 EDT
I have to disagree with the first two hunks, which appear to be
cosmetic.  Hunk 3 accepted, good catch!  Hunk 4 likewise.  Hunks 6,7,8
applied, will need additional fixes to handle errors returned.  Good
catch on hunk 9.  Hunk 10, good fix.  Hunk 11 looks cosmetic.  Hunks
13,14,15,16,17 are good catches.

I'm not sure there's much point to checking for EROFS errors from
access(), as the logic is going to cause a failure when we try to
modify the files anyway.
Comment 4 Steve Grubb 2004-04-13 08:48:24 EDT
>I'm not sure there's much point to checking for EROFS errors from
>access(), as the logic is going to cause a failure when we try to
>modify the files anyway.

While this may be true...why bother to ask for the superuser password
when that will ultimately fail? My personal beliefs are to inform the
user at the earliest chance so they might be able to remount the
filesystem or otherwise perform corrective action instead of typing a
password and then seeing a failure. Its not as important as the other
bugs pointed out.

As for the first two chunks...while it seems cosmetic, it would be
good to fix so the next person doing a code review doesn't have to
report the same thing.

I did not check to see if these bugs exist in earlier versions of
Libuser. You might want to look into RHL 9 before it is no longer
maintained as well as the Enterprise versions & Fedora Core 1. The
read() return code being caught in an unsigned variable could cause
someone to have a bad day.

Thanks for giving this your attention !
Comment 5 Bob Cochran 2004-05-31 00:36:49 EDT
I applied the patch as part of rebuilding libuser (the Fedora Core 2
release version) against openldap-2.2.11. I'm not sure whether my
openldap packages will prove stable or what will happen with the
libuser packages. I'll let you know. 

Comment 6 Steve Grubb 2004-07-27 15:08:04 EDT
Created attachment 102236 [details]
Current patch I'm testing

This patch has the error handling Nalin suggested. It compiles cleanly. I've
been running this for 2 months with no known problems.
Comment 7 Miloslav Trmač 2004-09-02 15:00:50 EDT
Thanks for the patch.

I have applied it, after fixing a few leaks introduced
by or found while applying this patch, properly initializing
struct lu_errror with a message, and most importantly
handling the other lu_make_crypted() call in modules/ldap.c
(otherwise setting a password when /dev/urandom is not
available would set an empty password).
Comment 8 Miloslav Trmač 2004-09-02 18:22:11 EDT
Should be fixed in libuser-0.51.10-1, when it hits rawhide.
Thanks again.

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