Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<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

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ