Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 543310 - <dev-util/diffball-1.0.1-r1: multiple stack buffer overflows
Summary: <dev-util/diffball-1.0.1-r1: multiple stack buffer overflows
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Security
Classification: Unclassified
Component: Vulnerabilities (show other bugs)
Hardware: All Linux
: Normal minor (vote)
Assignee: Gentoo Security
URL:
Whiteboard: B3 [noglsa]
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-14 13:57 UTC by Aidan Thornton
Modified: 2020-03-25 19:17 UTC (History)
3 users (show)

See Also:
Package list:
=dev-util/diffball-1.0.1-r2
Runtime testing required: ---
stable-bot: sanity-check+


Attachments
Example files (diffball-issues.zip,977 bytes, application/zip)
2015-03-14 13:57 UTC, Aidan Thornton
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Aidan Thornton 2015-03-14 13:57:31 UTC
Created attachment 398884 [details]
Example files

There are two trivial stack buffer overflows in diffball's parsers for bdelta and gdiff files:

* bdeltaReconstructDCBuff reads a uint8 size field and then reads three times that number of bytes from the input file into a 12-byte stack buffer, with no further checks:

        #define BUFF_SIZE 12
        unsigned char buff[BUFF_SIZE];
        ...
        cread(patchf, buff, 1);
        int_size = buff[0];
        ...
        cread(patchf, buff, 3 * int_size);

* gdiffReconstructDCBuff reads 13 bytes of input data into a 5-byte buffer. This is annoying to reproduce and probably isn't as exploitable. If *buff == 255 it effectively does this:

        unsigned char buff[5];
        ...
        ob=8; lb=4;
        cread(patchf, buff + 1, lb + ob)

Sample files demonstrating both of these issues are attached, use them like so:
patcher hello_kitty.jp2 diffball-differ-crash.bdelta /dev/null

I haven't reported this upstream because there doesn't appear to be much of an upstream to speak of; diffball was developed by a Gentoo developer, is mainly used by Gentoo (primarily in emerge-delta-webrsync) and upstream development appears dead.
Comment 1 Aidan Thornton 2015-03-15 00:16:15 UTC
Make that three, the xdelta1 parser has another one. This function is called with buff pointing to a 32-byte stack buffer:

unsigned long inline 
readXDInt(cfile *patchf, unsigned char *buff)
{
	unsigned long num=0;
	signed int count=-1;
	do {
		count++;
		cread(patchf, buff + count, 1);
	} while(buff[count] & 0x80);
	for(; count >= 0; count--) {
		num <<= 7;
		num |= (buff[count] & 0x7f); 
	}
	return num;
}


Also, it appears I can gain control of the instruction pointer using the bdeltaReconstructDCBuff buffer overflow. Stack smashing protection doesn't help much here - since dcbuff is allocated in the parent's stack frame and contains function pointers that are called by bdeltaReconstructDCBuff, we can overwrite them and gain control of EIP before the stack canary ever gets checked. Guessing this means it's an arbitrary code execution bug on non-hardened.
Comment 2 Kristian Fiskerstrand (RETIRED) gentoo-dev 2015-03-15 00:22:32 UTC
(In reply to Aidan Thornton from comment #0)

Thanks for the reports. Adding the maintainer for comment
Comment 3 Rafael Martins (RETIRED) gentoo-dev 2015-03-30 03:47:25 UTC
Hi Aidan,

FYI, we are working on it. Unfortunately I hadn't enough time to work on this properly yet, but I'll update you here when I have patches for the reported issues.

thanks
Comment 4 Rafael Martins (RETIRED) gentoo-dev 2015-04-27 05:18:00 UTC
CC'ing Brian Harring, diffball author
Comment 5 Brian Harring (RETIRED) gentoo-dev 2015-04-27 05:45:55 UTC
Looks of it someone took a fuzzer to the codebase....

Regarding the bdelta format, if you've not looked at the bdelta implementation in the tree I'd suggest so.  I'd assume it has a similar issue offhand.  Bdelta == deltup format- basically is dead as far as I know.

I'll take you at your word for the readXDInt bit; I don't think it's as trivial as it appears due to the high bit requirement, but that's probably just a matter of window dressing to bypass it.

Either way, the fixes are simple and a 1.0.2 can be cut pretty much now; I've already confirmed the overflows are fixed.

@gentoo security peeps, k_f in particular; you're aparently asking that the fixes be kept private until there is a sign off from Aidan?  Can you clarify that?
Comment 6 Kristian Fiskerstrand (RETIRED) gentoo-dev 2015-04-27 07:20:04 UTC
(In reply to Brian Harring from comment #5)
> Looks of it someone took a fuzzer to the codebase....
> 

Indeed, been popular recently, and a good thing, thanks :)

> Either way, the fixes are simple and a 1.0.2 can be cut pretty much now;
> I've already confirmed the overflows are fixed.

Very good.

> 
> @gentoo security peeps, k_f in particular; you're aparently asking that the
> fixes be kept private until there is a sign off from Aidan?  Can you clarify
> that?

(i) If a new release is cut from what is considered upstream of the package making the issues public we can make it public here as well (publication should include proper credits for discovery). 
(ii) If there is an external upstream we need to privately alert them. 
(iii) We also need to decide whether these fixes merits zero, one or more CVEs, what is your take on this?
Comment 7 Aidan Thornton 2015-07-20 21:58:36 UTC
Hey, sorry, I kind of lost track of what's happening with this. Could anyone fill me in. Again, sorry about this, I'm not normally in the security reporting business.
Comment 8 Rafael Martins (RETIRED) gentoo-dev 2015-07-21 00:27:18 UTC
(In reply to Aidan Thornton from comment #7)
> Hey, sorry, I kind of lost track of what's happening with this. Could anyone
> fill me in. Again, sorry about this, I'm not normally in the security
> reporting business.

From my last update, it seems that Brian (upstream maintainer) is working on patches.
Comment 9 Yury German Gentoo Infrastructure gentoo-dev 2015-07-21 00:36:28 UTC
(In reply to Brian Harring from comment #5)

> @gentoo security peeps, k_f in particular; you're aparently asking that the
> fixes be kept private until there is a sign off from Aidan?  Can you clarify
> that?

Brian, since you are the upstream maintainer of this code (unless I am wrong). You are free to let us know to make it public any time you would like to. 

We made it confidential until upstream (you) were notified and then can choose to release it publicly or keep it private until you have patches ready. 

As a side note, when you make it public we encourage  you to give Aidan credit for the discovery. We can also help request the CVE number, as part of publishing it to oss-security.
Comment 10 Yury German Gentoo Infrastructure gentoo-dev 2015-09-02 13:55:12 UTC
(In reply to Rafael Martins from comment #8) 
> From my last update, it seems that Brian (upstream maintainer) is working on
> patches.

Rafael do you have any updates on this?
Comment 11 Yury German Gentoo Infrastructure gentoo-dev 2015-11-03 17:25:35 UTC
Are we ready to make this public?
Comment 12 Yury German Gentoo Infrastructure gentoo-dev 2016-02-14 18:07:29 UTC
Two months have went by, is this public material now?

Also what about a fix? the patch is here.
Comment 13 Yury German Gentoo Infrastructure gentoo-dev 2016-04-23 03:04:40 UTC
This has been not fixed in a year. 
Please advise if this is to be fixed, or do we remove it from Tree.
Comment 14 Yury German Gentoo Infrastructure gentoo-dev 2016-06-05 22:24:23 UTC
Talked with rafaelmartins - He will try to contact upstream. If not successful will try to patch himself.
Comment 15 Yury German Gentoo Infrastructure gentoo-dev 2017-02-19 14:00:56 UTC
This bug has been here for two (2) years. 
Rafael, please either fix or drop from portage.
Comment 16 Aaron Bauman (RETIRED) gentoo-dev 2019-09-07 17:27:15 UTC
ping...
Comment 17 Thomas Deutschmann (RETIRED) gentoo-dev 2019-10-26 21:41:22 UTC
Rafael ack'ed in IRC that we can las-rite package. Treecleaners, please start your magic!
Comment 18 Rafael Martins (RETIRED) gentoo-dev 2019-10-26 21:44:08 UTC
(In reply to Thomas Deutschmann from comment #17)
> Rafael ack'ed in IRC that we can las-rite package. Treecleaners, please
> start your magic!

just confirming it here. I have no time/energy to work on this at this point. if someone wants to take care of it, that's the time to step up. sorry guys :(
Comment 19 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-10-27 07:42:54 UTC
$ rdep */diffball
== rdep of dev-util/diffball ==
app-arch/tarsync-0.2.1-r1
app-portage/distpatch-0.1.2-r1
app-portage/emerge-delta-webrsync-3.7.5
app-portage/emerge-delta-webrsync-3.7.6


The first one is unmaintained.  The second one is also Rafael's, so I suppose he'd be fine with it.  The third one is high-profile dev-portage package, so I don't think users will be happy seeing it gone.
Comment 20 Zac Medico gentoo-dev 2019-10-27 22:21:43 UTC
Sent upstream PR: https://github.com/rafaelmartins/diffball/pull/2

Sent gentoo PR: https://github.com/gentoo/gentoo/pull/13471
Comment 21 Larry the Git Cow gentoo-dev 2019-10-27 23:51:33 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=c1faef3a67ae5904fcd8e95495b005e268f1bc76

commit c1faef3a67ae5904fcd8e95495b005e268f1bc76
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2019-10-27 22:14:09 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2019-10-27 23:51:03 +0000

    dev-util/diffball: revbump to 1.0.1-r1 for bug 543310
    
    Fix stack buffer overflows reported in bug 543310.
    
    Bug: https://bugs.gentoo.org/543310
    See: https://github.com/zmedico/diffball/pull/1
    Reported-by: Aidan Thornton <makosoft@googlemail.com>
    Package-Manager: Portage-2.3.78, Repoman-2.3.17
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 dev-util/diffball/Manifest                 |  2 ++
 dev-util/diffball/diffball-1.0.1-r1.ebuild | 38 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)
Comment 22 Agostino Sarubbo gentoo-dev 2020-03-17 18:44:36 UTC
amd64 stable
Comment 23 Agostino Sarubbo gentoo-dev 2020-03-18 11:11:22 UTC
ppc stable
Comment 24 Agostino Sarubbo gentoo-dev 2020-03-18 12:03:49 UTC
ia64 stable
Comment 25 Agostino Sarubbo gentoo-dev 2020-03-18 16:03:41 UTC
x86 stable.

Maintainer(s), please cleanup.
Security, please vote.
Comment 26 Larry the Git Cow gentoo-dev 2020-03-18 16:14:58 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=31862fbc2744fd8fda8a30611daafe22ed97dbf5

commit 31862fbc2744fd8fda8a30611daafe22ed97dbf5
Author:     Zac Medico <zmedico@gentoo.org>
AuthorDate: 2020-03-18 16:11:59 +0000
Commit:     Zac Medico <zmedico@gentoo.org>
CommitDate: 2020-03-18 16:14:49 +0000

    dev-util/diffball: remove vulnerable versions (bug 543310)
    
    Bug: https://bugs.gentoo.org/543310
    Package-Manager: Portage-2.3.94, Repoman-2.3.21
    Signed-off-by: Zac Medico <zmedico@gentoo.org>

 dev-util/diffball/Manifest                 |  1 -
 dev-util/diffball/diffball-1.0.1-r1.ebuild | 38 ------------------------------
 dev-util/diffball/diffball-1.0.1.ebuild    | 34 --------------------------
 3 files changed, 73 deletions(-)
Comment 27 Thomas Deutschmann (RETIRED) gentoo-dev 2020-03-25 19:17:11 UTC
GLSA Vote: No

Repository is clean, all done!