[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 30 Jul 2015 10:05:26 +0200
From: Stefan Cornelius <scorneli@...hat.com>
To: cve-assign@...re.org
Cc: oss-security@...ts.openwall.com
Subject: Re: Re: CVE Request: Multiple vulnerabilities in
freexl 1.0.0g
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Mon, 6 Jul 2015 12:49:45 +0200
Stefan Cornelius <scorneli@...hat.com> wrote:
> -----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?
Any update here?
> 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).
This is fixed in version 1.0.2. Can I get a CVE?
Thanks,
- --
Stefan Cornelius / Red Hat Product Security
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBCAAGBQJVudrHAAoJEETwiYCjVSmPLW0H/0fWa6u236XbEA7Wf9ycSaWV
aRvo/PiZjwSZmtDvaGRadNi9AT9xnhYVtFBit/ZIwr/vQaBRR0qr4tMsCefrnqfu
Ga2PztrgWtDt9PpO+YHmnZgLNF9ZlrHHiB7n4gTnNd+NhCHbzHCgh2f+sMDs4hdW
gpZr5mMhq3hFWX7XZGNCQ+f9yi6a7k8kE71D090KjYho6tt4q+VnwS+6+i7Lv+St
waompN/EI1RiG2+TkISpA5IsNrUmkDarmRbg1tnBnmbcqGEh1sb8Lgr52z74qUQq
y9jsR9pAxGV88aw8VJ7EB7Ed4+/aQA9rAlsk6bq3Q9qn9EJ9YhMVdNoRT1W1O3M=
=MUuo
-----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.
