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

[a.furieri () lqt it]

On Mon, 6 Jul 2015 12:49:45 +0200, Stefan Cornelius wrote:
>

Hi Stefan,

if I understand well your tests are based on the obsolete FreeXL 1.0.0g
that is not the most recent version available.

version 1.0.1 was released on 2015-03-22, and is exactly intended to
fix several critcal bugs dentified by American Fuzzy Lop when parsing
purposely malformed input files.

it could be surely usefull to learn if after switching to the more
recent version you still continue to confirm your issues.
and if the answer is eventually yes, sensing a copy of the input
files causing malfunctions will surely help to debug the code.

bye Sandro



> -----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?
>
> 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-----