server and client side remote code execution throu gh a buffer overflow in all git versions before 2.7.1 (un published ᴄᴠᴇ-2016-2324 and ᴄᴠᴇ ‑2016‑2315)

Laël Cellier <[email protected]> Wed, 16 Mar 2016 01:31:47 +0100
Newsgroups gmane.comp.security.fulldisclosure,gmane.comp.security.bugtraq,gmane.comp.security.basics,gmane.comp.security.full-disclosure
Message-ID <[email protected]>
This is a multi-part message in MIME format.
--------------E902092C459C0A63F18B895F
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable

Hello, original report describing the first overflow full details is=20
here http://pastebin.com/UX2P2jjg or at the attachment
The aim is to push a crafted tree object if the target is a server or=20
make a client cloning a crafted repository.

On 11/02/2016 16:50, Jeff King wrote this on the git security mailing lis=
t:
> On Thu, Feb 11, 2016 at 02:31:49PM +0100, 'La=C3=ABl Cellier' via Git=20
> Security wrote:
>> Ok the bug works by pushing or cloning a repository with a large
>> filename or a large number of nested trees.
>> [...]
>> The point is affected versions are still shipped as part of many
>> distributions as part of their stable branch, so I think it=E2=80=99s
>> important to get a =E1=B4=84=E1=B4=A0=E1=B4=87 for public awareness.
> Yes, I do think versions below v2.7.0 have a heap overflow, as you
> mentioned. But I don't think that is the only problem with path_name(),
> even in the current version.
>
> I'll repeat the code here (the version you posted was indented badly,
> and I had trouble reading it):
>
> -- >8 --
> char *path_name(const struct name_path *path, const char *name)
> {
>          const struct name_path *p;
>          char *n, *m;
>          int nlen =3D strlen(name);
>          int len =3D nlen + 1;
>
>          for (p =3D path; p; p =3D p->up) {
>                  if (p->elem_len)
>                          len +=3D p->elem_len + 1;
>          }
>          n =3D xmalloc(len);
>          m =3D n + len - (nlen + 1);
>          memcpy(m, name, nlen + 1);
>          for (p =3D path; p; p =3D p->up) {
>                  if (p->elem_len) {
>                          m -=3D p->elem_len + 1;
>                          memcpy(m, p->elem, p->elem_len);
>                          m[p->elem_len] =3D '/';
>                  }
>          }
>          return n;
> }
> -- 8< --
>
> The problem you describe is one where the size of the allocation does
> not match what strcpy would write. And that's kind-of fixed by moving t=
o
> memcpy() in 34fa79a6, because at least now the initial value of "len"
> matches the number of bytes we write (so that number might be totally
> bogus, but we don't write more than we allocate).
>
> But "len" can also change after the fact, due to the loop. If you have =
a
> sequence of path components, each less than 2^31, they can sum to a muc=
h
> smaller positive value due to integer overflow (e.g., A/B/C with length=
s
> A=3D2^31-5, B=3D2^31-5, C=3D20 would yield len=3D10). Then the buffer i=
s too
> small to fit C, let alone all of the extra components we insert in the
> second loop.
>
> The fix I came up with for this is to convert all of the "int" variable=
s
> here to "size_t". That doesn't actually _fix_ the problem at all, but
> does mean on a 64-bit system that you need a 2^64-long path to trigger
> it, which is impractical. But that doesn't help 32-bit systems (though
> in practice, I wouldn't be surprised if we barf long before that, as we
> would be unable to hold the "struct name_path" list in memory).
>
> Note that there is also a similar problem in tree-diff.c's
> path_appendnew().  There we build up the full pathname in a strbuf,
> which checks for overflow. But we then pass that length as an int and
> allocate a FLEX_ARRAY struct with it, which can end up too-small. This
> one is the more interesting of the two, I think, as it triggers via
> git-log, whereas the path_name() happens only during a repack (so it
> will hit you _eventually_, but probably not as soon as you've cloned).
>
> My solution there was similar: use size_t, which at least means you'd
> have to allocate petabytes on a 64-bit system to trigger it (much less
> on a 32-bit system, but _probably_ you'd be saved by malloc failing
> first).
>
> And that's why I dragged my feet on sending those fixes upstream; I
> don't think they're complete. The complete fix would be to use size_t
> consistently to store return values for strlen(), and to do integer
> overflow checks whenever we do computations on size_t.
>
> Those of you on this list may recall I posted a series for the latter
> last year, but it was somewhat invasive. It may be worth resurrecting.
>
> I think we could also get rid of path_name() entirely. The sole purpose
> at this point is to compute the name-hash for pack-objects, which could
> be done by walking the name_path list rather than re-constructing the
> whole thing in memory.
>
> -Peff
Of course everything Peff talked about above is now fixed in git 2.7.1=20
with the removal of path_name() and the size_t/overflow check in=20
tree-diff.c. It was even fixed earlier for users of github enterprise.
However, several months after the last message on this thread, I=E2=80=99=
m not=20
aware of any Linux distribution that issued a fix for their stable=20
branch. Last week I could contact wikimedia so they could fix their=20
gerrit=E2=80=91gc server. Bitbucket, GitLab still suffer from that issue =
(they=20
even use a git version before=20
git/commit/34fa79a6cde56d6d428ab0d3160cb094ebad3305 which is the easiest=20
one to trigger because of strcpy() instead of memcpy() ).
Users of gerrit are also affected due to gerrit=E2=80=91gc (so even proba=
bly=20
google=E2=80=99s servers). But as the frontend is Jgit, the installed git=
=20
version number is hidden (so the only way is to exploit remote code=20
execution).
While it seems normal the =E1=B4=84=E1=B4=A0=E1=B4=87 details are still u=
npublished, I=20
definitely can=E2=80=99t deal with every major provider. Their probably m=
any=20
popular git services which are still impacted, I just can=E2=80=99t check=
 for=20
everyone of them (for example I didn=E2=80=99t checked sourceware.org the=
 site=20
that allow to download glibc)

People surely remember=20
https://www.google.com/search?tbm=3Dnws&q=3DCVE-2014-9390 breaking the ne=
ws=20
about a similar issue in that software (which allowed most distros to=20
fix it quikcly). It seems while this threat is more widespread, it=20
definitely lacks advertisement. I reported it to cert in December but=20
didn=E2=80=99t got any response except the automated acknowledgement.
So some peoples suggested me to post about it at security mailing lists.

Those buffer overflows are reported simply as memory corruptions. While=20
I=E2=80=99m sure affected variables aren=E2=80=99t at the end of allocate=
d heap, I=20
definitely lack the required skills to produce a proof for remote code=20
execution http://security.stackexchange.com/q/117394/36301 (I know how=20
to put arbitrary data in paths but I don=E2=80=99t know how to exploit a =
heap=20
overflow even without aslr and dep). Being too lazy, I didn=E2=80=99t get=
 the=20
required mark to go at the university which could have taught it (I had=20
to go at an another one). So I won=E2=80=99t write it.
So if someone is interested in producing such proof=E2=80=AF=E2=80=A6 Ple=
ase do it=E2=80=AF=E2=80=A6


But the primary concern is how to solve that advertisement problem=E2=80=AF=
? Any=20
ideas=E2=80=AF?

--------------E902092C459C0A63F18B895F
Content-Type: text/plain; charset=UTF-8;
 name="cve-2016-2315.txt"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="cve-2016-2315.txt"

Ly8gSW4gcmV2aXNpb24uYyBiZWZvcmUgaHR0cHM6Ly9naXRodWIuY29tL2dpdC9naXQvY29t
bWl0LzM0ZmE3OWE2Y2RlNTZkNmQ0MjhhYjBkMzE2MGNiMDk0ZWJhZDMzMDUNCmNoYXIgKnBh
dGhfbmFtZShjb25zdCBzdHJ1Y3QgbmFtZV9wYXRoICpwYXRoLCBjb25zdCBjaGFyICpuYW1l
KSAvLyBieSBkZXNpZ24sIG5hbWVfcGF0aC0+bGVuIGlzIGEgMzIgYml0cyBpbnQsIGJ1dCB0
aGlzIGRvZXNuJ3QgY29uY2VybiBuYW1lDQp7DQogICAgICBjb25zdCBzdHJ1Y3QgbmFtZV9w
YXRoICpwOw0KICAgICAgY2hhciAqbiwgKm07DQogICAgICBpbnQgbmxlbiA9IHN0cmxlbihu
YW1lKTsgLy8gdGhlIHNpemUgaXMgY29udmVydGVkIHRvIGEgcG9zaXRpdmUgbnVtYmVyICh0
aGUgY29ycmVjdCBzaXplIHdhcyBhbGxvY2F0ZWQgcHJldmlvdXNseSB3aXRoIGFuIHVuc2ln
bmVkIGxvbmcpLiBJIGdvdCA3MDU4MDQxMDANCiAgICAgIGludCBsZW4gPSBubGVuICsgMTsN
Cg0KICAgICAgZm9yIChwID0gcGF0aDsgcDsgcCA9IHAtPnVwKSB7IC8vbG9vcCBpcyBza2lw
cGVkIChleGNlcHQgZm9yIHRoZSBjdmUtMjAxNi0yMzI0IGNhc2Ugd2hpY2ggaXMgZml4ZWQg
c2luY2UgMi43LjEgaW4gRmVicnVhcnkgMjAxNikNCiAgICAgICAgICBpZiAocC0+ZWxlbV9s
ZW4pDQogICAgICAgICAgICAgIGxlbiArPSBwLT5lbGVtX2xlbiArIDE7DQogICAgICB9DQog
ICAgICBuID0geG1hbGxvYyhsZW4pOyAvLyBpZiBsZW4gaXMgbmVnYXRpdmUsIGl0IHdpbGwg
YWxzbyBiZSBjb252ZXJ0ZWQgdG8gYSBuZWdhdGl2ZSA2NCBiaXRzIGludGVnZXIgKih3aGlj
aCBleHBsYWlucyBpdCBpcyBub3JtYWxseSB0cnlpbmcgdG8gYWxsb2NhdGUgc2VydmVyYWwg
UGIgb2YgcmFtIG1vc3Qgb2YgdGhlIHRpbWUpKiB3aGljaCB3aWxsIGJlIHJlYWQgYXMgcG9z
aXRpdmUgYWZ0ZXIgdGhhdC4gLy8gYnV0IHRoaXMgaXNuJ3QgdGhlIHJ1biBjYXNlIHRoYXQg
aXMgaW50ZXJlc3RpbmcgaGVyZS4NCiAgICAgIG0gPSBuICsgbGVuIC0gKG5sZW4gKyAxKTsg
Ly8gdGhlIHNpemUgb2YgbSBpcyBsb3dlciB0aGFuIG5hbWUNCiAgICAgIHN0cmNweShtLCBu
YW1lKTsgLy8gc3RyY3B5IHJlbHkgb24gdGhlIG51bGwgdGVybWluYXRpbmcgY2hhcmFjdGVy
LiBUaGUgcmVzdWx0IGlzIHdyaXR0ZW4gaW4gYW4gdW5hbGxvY2F0ZWQgbWVtb3J5IGZyb20g
aGVhcC4gVGhpcyBpcyB0aGUgZGVmaW5pdGlvbiBvZiBoZWFwIG92ZXJmbG93IGVuYWJsaW5n
IHNlcnZlciBzaWRlIHJlbW90ZSBjb2RlIGV4ZWN1dGlvbiBpZiBuYW1lW10gY29udGFpbnMg
YXNzZW1ibHksIGFuZCBoYXZlIHRoZSBjb3JyZWN0IHNpemUuIFRoaXMgb3BlbiB0aGUgd2F5
IHRvIGRlZmVhdCBjYW5hcmllcyBhc2xyLCBhbmQgbnggY29tYmluZWQgc2VlIGh0dHA6Ly9z
ZWN1cml0eS5zdGFja2V4Y2hhbmdlLmNvbS9xLzIwNDk3LzM2MzAxI2NvbW1lbnQxODIwMDRf
MjA1NTANCiAgICAgIGZvciAocCA9IHBhdGg7IHA7IHAgPSBwLT51cCkgew0KICAgICAgICAg
IGlmIChwLT5lbGVtX2xlbikgew0KICAgICAgICAgICAgICBtIC09IHAtPmVsZW1fbGVuICsg
MTsNCiAgICAgICAgICAgICAgbWVtY3B5KG0sIHAtPmVsZW0sIHAtPmVsZW1fbGVuKTsNCiAg
ICAgICAgICAgICAgbVtwLT5lbGVtX2xlbl0gPSAnLyc7DQogICAgICAgICAgfQ0KICAgICAg
fQ0KICAgICAgcmV0dXJuIG47DQp9DQo=
--------------E902092C459C0A63F18B895F
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline