[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150706124945.3b70a6fa@redhat.com>
Date: Mon, 6 Jul 2015 12:49:45 +0200
From: Stefan Cornelius <scorneli@...hat.com>
To: cve-assign@...re.org
Cc: jodie.cunningham@...il.com, oss-security@...ts.openwall.com,
a.furieri@....it
Subject: Re: Re: CVE Request: Multiple vulnerabilities in
freexl 1.0.0g
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Fri, 27 Mar 2015 19:48:01 -0400 (EDT)
cve-assign@...re.org wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> >> #4: FreeXL 1.0.0g did not properly check requests for workbook
> >> memory allocation. A specially crafted input file could cause a
> >> Denial of Service, or possibly write onto the stack.
>
> > This vulnerability is related to the missing "> 1024 * 1024" test in
> > the parse_SST function.
>
> Use CVE-2015-2776.
>
>
> >>> #2: A flaw was found in the function allocate_cells(). A specially
> >>> crafted file with invalid workbook dimensions could possibly
> >>> result in stack corruption near freexl.c:1074
>
> >> Does this refer to the missing "== NULL" tests within the
> >> allocate_cells function?
>
> > Yes
>
> >> Is a NULL pointer dereference going to occur
> >> before the code reaches a point where there can be stack
> >> corruption?
>
> > I don't believe so. It looks like these are initialized as NULL, and
> > if they are still NULL at this point in execution then we assume the
> > input file was malformed and exit with the appropriate return code.
>
> In that case, we don't know what vulnerability you mean for #2.
>
> Between the unpatched code and the patched code, the only change in
> the allocate_cells function is the addition of checks for whether
> workbook or workbook->active_sheet is NULL. In the unpatched code, if
> either of these were NULL, workbook->active_sheet->rows would result
> in a NULL pointer dereference. As far as we know, this outcome is not
> typically described as "stack corruption."
>
> If the design of the allocate_cells function was supposed to
> anticipate that callers might provide a NULL value for workbook or
> workbook->active_sheet, then the unpatched code had a vulnerability in
> the allocate_cells function that might loosely be described as a "NULL
> pointer dereference vulnerability."
>
> We think you may mean that, in some cases, stack corruption has
> occurred because of invalid workbook dimensions before the
> allocate_cells function is called. In some or all of these cases, a
> side effect of the stack corruption is that either workbook or
> workbook->active_sheet is NULL. The patched code, instead of
> preventing the stack corruption (or detecting the stack corruption
> before calling allocate_cells), chooses to use these "== NULL" tests
> to infer that stack corruption has occurred. Is this correct?
Hi,
It seems like this still has no CVE, apparently because the exact
details of this issue are unclear. I'll try to clear up the situation
and will also provide details for another, new issue below.
Further info for "issue #2":
============================
The common_open() function initializes the workbook (at that point,
most interesting members are NULL). A bit further down, it parses
all the biff records via the loop around read_biff_next_record():
> while (1)
> {
> int ret = read_biff_next_record (workbook, swap, &errcode);
> if (ret == -1)
> break; /* EOF */
> if (ret == 0)
> goto stop;
> }
After parsing all the records, the workbook->first_sheet member
points to something valid, but workbook->active_sheet does not,
it's still NULL.
common_open() has a check for first_sheet, but since the
allocate_cells() function operates on the workbook->active_sheet
member, so we ultimately get a NULL pointer dereference in
allocate_cells(). I've not seen any indication of a stack
corruption.
> p_sheet = workbook->first_sheet;
> while (p_sheet)
> {
> if (p_sheet->valid_dimension == 0)
> {
> /* setting Sheet dimensions */
> int ret;
> p_sheet->rows += 1;
> p_sheet->columns += 1;
> ret = allocate_cells (workbook);
Does that clear the situation up enough to assign a CVE to this?
New issue: allocate_cells() integer overflow
============================================
There's an integer overflow in the allocate_cells() function
when trying to allocate the memory for worksheet with specially
crafted row/column dimensions. This can be exploited to cause a
heap memory corruption. The most likely outcome of this is a crash
when trying to initialize the cells later in the function.
> workbook->active_sheet->cell_values =
> malloc (sizeof (biff_cell_value) *
> (workbook->active_sheet->rows *
> workbook->active_sheet->columns));
I've not assigned a CVE to this, so I'm hereby requesting one (mainly
because this thread is a bit old and the problem is fairly close to the
patched code, so there may be a slim chance that somebody else noticed
this independently and requested a CVE for this in private).
I've CCed the maintainer to this mail.
Thanks and kind regards,
- --
Stefan Cornelius / Red Hat Product Security
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJVml1KAAoJEETwiYCjVSmPiPAH/0LcBh/EBFJvZARebc5uyBNg
azHfurdkGBSSOnkSbywePGdJ0hxttzaaLtmu5H/pnJTksW8LgeIC53/+/Bi83YNX
hMvRiiVZBhl1qbnvU95BuykoLmaetCt0CkwcnfFm7Fqx5+r+leE/RXEGm4D6NyPR
jfyEOT2/Y736OM/cASSaE8gw0ypWada44rRfLisvFk1afPp2RPY0rqUHpCXaD6Vk
NR96Lli/XZS/g3p1wEQMsoA+DZbuu7IqFu89PZbEvrOOawEIZcn/bec83vRQVq+T
wLYpxagvzkQ0FIPHLFuTSM+/OCwWGgzi4AGVimvt2O3oQqo6BMwN9avp4R9N9vo=
=o+1a
-----END PGP SIGNATURE-----
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
