Re: Re: CVE Request: Multiple vulnerabilities in freexl 1.0.0g

[Stefan Cornelius <scorneli () redhat com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Mon, 6 Jul 2015 12:49:45 +0200
Stefan Cornelius <scorneli () redhat com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> On Fri, 27 Mar 2015 19:48:01 -0400 (EDT)
> cve-assign () mitre 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-----