Bug 143675 - kernel_read() improper return validatio
Summary: kernel_read() improper return validatio
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Red Hat Enterprise Linux 3
Classification: Red Hat
Component: kernel
Version: 3.0
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dave Anderson
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2004-12-23 19:01 UTC by Josh Bressers
Modified: 2007-11-30 22:07 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-01-04 20:09:03 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Josh Bressers 2004-12-23 19:01:25 UTC
This message was posted to full disclosure on 2004-12-16

http://lists.netsys.com/pipermail/full-disclosure/2004-December/030060.html



To quote the first paragraph:

[2.] Checking whether the return value from kernel_read() is
non-negative is not enough. kernel_read() function returns the number of
bytes that have actually been read, which could be different from the
number of bytes requested to be read (indicated by the fourth
parameter). If the number of bytes read differs from the number of bytes
requested, and an appropriate check is omitted, the wrong assumption is
made as to what the contents of the destination buffer used for reading
are. This could lead to unexpected / non-deterministic program behavior,
which is especially unpleasant for the kernel. We have noticed the same
problem in several different places. Considering the fact that in other
cases, the return from kernel_read() is correctly validated, these are
definite bugs.



I don't know if this is a security issue or not.  I don't know enough about the
kernel to make that judgement.  I can this as a potential DoS, but priv
escalataion seems unlikely.


If this issue is a security issue, I'll file appropriate bugs against our other
kernels.

Comment 1 Ernie Petrides 2004-12-23 19:57:47 UTC
The RHEL3 code base is significantly different from 2.6 in these areas,
but a cursory glance indicates that we do have similar problems in RHEL3.


Comment 2 Dave Anderson 2005-01-03 21:19:20 UTC
In my opinion, the claims made by this bugzilla are bogus.

During an exec operation, do_execve() reads the first 128 bytes 
(BINPRM_BUF_SIZE) of the file being executed are read into the buf[] 
array of a linux_binprm structure:

/*
 * This structure is used to hold the arguments that are used when loading
binaries.
 */
struct linux_binprm{
        char buf[BINPRM_BUF_SIZE];
        struct page *page[MAX_ARG_PAGES];
        unsigned long p; /* current top of mem */
        int sh_bang;
        struct file * file;
        int e_uid, e_gid;
        kernel_cap_t cap_inheritable, cap_permitted, cap_effective;
        int argc, envc;
        char * filename;        /* Name of binary as seen by procps */
        char * interp;          /* Name of the binary really executed.
Most
                                   of the time same as filename, but
could be
                                   different for binfmt_{misc,script} */
        unsigned long loader, exec;
};

The 128 bytes read from the file or script (BINPRM_BUF_SIZE) appears
to be a "reasonable" value that covers all bases for getting started,
i.e., that many bytes are read from the file to determine what kind
of file it is, be it an ELF executable, a shell script, or whatever. 

After the kernel_read() or prepare_binprm() function does the 128-byte
read, the linux_binrpm structure is subsequently passed to the
search_binary_handler() function, which walks through the currently
supported set of handler functions, passing each the linux_binprm
structure.  Eventually one of the handlers makes sense out of the 
contents of the buf[] array, after going through its own set of 
extensive verifications on the contents of the buffer.  Because all 
executables may be sent to all of the current set of handlers, those
handlers are very specific about verifying the contents of those first
bytes.  (In reality, looking at my RHEL3 kernel, there are only two
handlers, load_elf_binary() which gets call first, and then the 
load_script() function gets called second...)

Anyway, the actual number of bytes initially read is not what's
important here.  In fact, one of the supposed "bugs" in the report
is dead wrong, which is in this piece of do_execve(): 

> 3) fs/exec.c:
                                                                     
                                                                     
                 >
> ...
> 1132:   retval = prepare_binprm(bprm);
> 1133:   if (retval < 0)
> 1134:           goto out;
> ...
                                                                     
                                                                     
                 >
> Here, return value from prepare_binprm() is assigned to retval on
> line 1132, and then on line 1133 retval is incorrectly validated.

This is an incredibly bogus claim.  For example, a shell script of
the form:

#!/bin/sh
ls

has 13 bytes, clearly less than 128 (BINPRM_BUF_SIZE) bytes that the 
bugzilla claims should be "validated".  Again, the buf[] array will
end up containing the 13 bytes above (the read will return 13 instead
of 128).  The search_binary_handler() function will first pass it to 
load_elf_binary() -- which will fail because those 13 bytes are 
clearly not going to pass muster as an ELF header -- and then pass it 
to load_script(), which will verify the #! at the beginning, parse 
the interpreter name, etc., and then "/bin/sh" gets executed instead.  

Again, the validation is done by the binary handlers on the contents
of the buffer, whatever they might be.  Just because there's less 
than 128 bytes read certainly does not imply a failure.



Comment 3 Josh Bressers 2005-01-04 19:57:54 UTC
Dave,

If you feel this is a non issue, feel free to close this bug.  From your
previous comment, I'm under the impression that this is not a security issue in
the least bit.

Thanks.

Comment 4 Dave Anderson 2005-01-04 20:09:03 UTC
That's right -- they seem to be jumping to conclusions based on
prior security case issues (the BZ #'s escape me) where the
return from kernel_read() did need more validating.  But those
cases were in functions that were designed to do the validation,
whereas in this case, the calls are simply to read the beginning
of the file in "preparation" for subsequent validation of the
contents read.


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