VULNERABILITY DETAILS
Malform patch file may access heap out of bound.

This is just implementation flaw of mbspatch and should be guarded earlier (say, patch file must be trusted). And this is not heap arbitrary write, so I'm not sure this is still considered a security issue. Just in case.

I'm talking about this file https://cs.chromium.org/chromium/src/third_party/bspatch/mbspatch.cc?q=mbspatch&sq=package:chromium&l=1

Two issues,
1. MBSPatchTriple.z is int. And we didn't check if it is negative. Thus,
180    fbuffer += ctrlsrc->z
will move fbuffer pointer to anywhere before fbuffer. Later, with

153    diffsrc[i] += fbuffer[i];
which means read access to almost arbitrary address.

2. MBSPatchHeader.cblen may be not multiple of sizeof(MBSPatchTriple).
so,
123    unsigned char *diffsrc = buf + header->cblen;
diffsrc could be overlapped with ctrlsrc.

with
153    diffsrc[i] += fbuffer[i];
We can overwrite partial value of ctrlsrc. For example, let cblen=3, diffsrc[0] is last byte of ctrlsrc->x. In order words, the check in line 147 is bypassed.
147    if (fbuffer + ctrlsrc->x > fbufend

then, with
152    for (unsigned int i = 0; i < ctrlsrc->x; ++i) {
153      diffsrc[i] += fbuffer[i];
154    }
we can overflow diffsrc beyond its original allocation.


VERSION
Chrome Version: ToT
Operating System: Windows

REPRODUCTION CASE
Here I demo the second issue. I don't have windows environment, so I compile my PoC with defines for linux.
$ cd third_party/bspatch
$ c++ -g -o main main.cc mbspatch.cc -D'CrcGenerateTable()' -I. -D'CrcCalc(a,b)=~0' -D'_wopen(a,b)=open(a,b,0664)' -Dwchar_t=char
$ ./main old.bin patch.bin new.bin
segmentation fault

FYI, following are values in my sample file:
header: slen=1, cblen=3, difflen=1, extralen=1
ctrlsrc[0].x = 1
the old.bin file is one bye: 0xf0.
After first ctrlsrc overwrite, x becomes 0xf0000001.


gdb log
Program received signal SIGSEGV, Segmentation fault.
0x0000000000400b70 in MBS_ApplyPatch (header=0x7fffffffccf0, patchfd=3, fbuffer=0x603010 <incomplete sequence \360>, filefd=5) at mbspatch.cc:153
153             diffsrc[i] += fbuffer[i];
(gdb) p i
$1 = 135117
(gdb) p fbufend - fbuffer
$2 = 1
(gdb) p header->cblen + header->difflen + header->extralen       # allocation size
$3 = 5
(gdb) bt
#0  0x0000000000400b70 in MBS_ApplyPatch (header=0x7fffffffccf0, patchfd=3, fbuffer=0x603010 <incomplete sequence \360>, filefd=5) at mbspatch.cc:153
#1  0x0000000000400ef6 in ApplyBinaryPatch (old_file=0x7fffffffd2d3 "old.bin", patch_file=0x7fffffffd2db "patch.bin", new_file=0x7fffffffd2e5 "new.bin") at mbspatch.cc:266
#2  0x0000000000400818 in main (argc=4, argv=0x7fffffffce38) at main.cc:4


 

Deleted: old.bin 1 bytes

old.bin 1 bytes Download

Deleted: patch.bin 37 bytes

patch.bin 37 bytes Download

Deleted: main.cc 119 bytes

main.cc 119 bytes View Download