SECURITY. From upstream:

"Multiple issues found in mpatch.c with a fuzzer:

- OVE-20180430-0001
- OVE-20180430-0002
- OVE-20180430-0004

With the following fixes:

- mpatch: be more careful about parsing binary patch data (SEC)
- mpatch: protect against underflow in mpatch_apply (SEC)
- mpatch: ensure fragment start isn't past the end of orig (SEC)
- mpatch: fix UB in int overflows in gather() (SEC)
- mpatch: fix UB integer overflows in discard() (SEC)
- mpatch: avoid integer overflow in mpatch_decode (SEC)
- mpatch: avoid integer overflow in combine() (SEC)

No exploits are known at the time."
This commit is contained in:
juanfra 2018-06-09 16:44:45 +00:00
parent f22cab42ca
commit 61959b1e02
2 changed files with 228 additions and 1 deletions

View File

@ -1,4 +1,4 @@
# $OpenBSD: Makefile,v 1.85 2018/04/25 09:40:07 juanfra Exp $
# $OpenBSD: Makefile,v 1.86 2018/06/09 16:44:45 juanfra Exp $
COMMENT-main = fast, lightweight source control management
COMMENT-x11 = graphical tooling for mercurial
@ -8,6 +8,7 @@ COMMENT-x11 = graphical tooling for mercurial
# Test also with:
# devel/py-hg-git
MODPY_EGG_VERSION = 4.5.3
REVISION = 0
DISTNAME = mercurial-${MODPY_EGG_VERSION}
CATEGORIES = devel

View File

@ -0,0 +1,226 @@
$OpenBSD: patch-mercurial_mpatch_c,v 1.2 2018/06/09 16:44:45 juanfra Exp $
Index: mercurial/mpatch.c
--- mercurial/mpatch.c.orig
+++ mercurial/mpatch.c
@@ -20,6 +20,7 @@
of the GNU General Public License, incorporated herein by reference.
*/
+#include <limits.h>
#include <stdlib.h>
#include <string.h>
@@ -27,6 +28,15 @@
#include "compat.h"
#include "mpatch.h"
+/* VC9 doesn't include bool and lacks stdbool.h based on cext/util.h */
+#if defined(_MSC_VER) || __STDC_VERSION__ < 199901L
+#define true 1
+#define false 0
+typedef unsigned char bool;
+#else
+#include <stdbool.h>
+#endif
+
static struct mpatch_flist *lalloc(ssize_t size)
{
struct mpatch_flist *a = NULL;
@@ -60,6 +70,35 @@ static ssize_t lsize(struct mpatch_flist *a)
return a->tail - a->head;
}
+/* add helper to add src and *dest iff it won't overflow */
+static inline bool safeadd(int src, int *dest)
+{
+ if ((src > 0) == (*dest > 0)) {
+ if (*dest > 0) {
+ if (src > (INT_MAX - *dest)) {
+ return false;
+ }
+ } else {
+ if (src < (INT_MIN - *dest)) {
+ return false;
+ }
+ }
+ }
+ *dest += src;
+ return true;
+}
+
+/* subtract src from dest and store result in dest */
+static inline bool safesub(int src, int *dest)
+{
+ if (((src > 0) && (*dest < INT_MIN + src)) ||
+ ((src < 0) && (*dest > INT_MAX + src))) {
+ return false;
+ }
+ *dest -= src;
+ return true;
+}
+
/* move hunks in source that are less cut to dest, compensating
for changes in offset. the last hunk may be split if necessary.
*/
@@ -70,17 +109,36 @@ static int gather(struct mpatch_flist *dest, struct mp
int postend, c, l;
while (s != src->tail) {
- if (s->start + offset >= cut)
+ int soffset = s->start;
+ if (!safeadd(offset, &soffset))
+ break; /* add would overflow, oh well */
+ if (soffset >= cut)
break; /* we've gone far enough */
- postend = offset + s->start + s->len;
+ postend = offset;
+ if (!safeadd(s->start, &postend) ||
+ !safeadd(s->len, &postend)) {
+ break;
+ }
if (postend <= cut) {
/* save this hunk */
- offset += s->start + s->len - s->end;
+ int tmp = s->start;
+ if (!safesub(s->end, &tmp)) {
+ break;
+ }
+ if (!safeadd(s->len, &tmp)) {
+ break;
+ }
+ if (!safeadd(tmp, &offset)) {
+ break; /* add would overflow, oh well */
+ }
*d++ = *s++;
} else {
/* break up this hunk */
- c = cut - offset;
+ c = cut;
+ if (!safesub(offset, &c)) {
+ break;
+ }
if (s->end < c)
c = s->end;
l = cut - offset - s->start;
@@ -114,15 +172,39 @@ static int discard(struct mpatch_flist *src, int cut,
int postend, c, l;
while (s != src->tail) {
- if (s->start + offset >= cut)
+ int cmpcut = s->start;
+ if (!safeadd(offset, &cmpcut)) {
break;
+ }
+ if (cmpcut >= cut)
+ break;
- postend = offset + s->start + s->len;
+ postend = offset;
+ if (!safeadd(s->start, &postend)) {
+ break;
+ }
+ if (!safeadd(s->len, &postend)) {
+ break;
+ }
if (postend <= cut) {
- offset += s->start + s->len - s->end;
+ /* do the subtraction first to avoid UB integer overflow
+ */
+ int tmp = s->start;
+ if (!safesub(s->end, &tmp)) {
+ break;
+ }
+ if (!safeadd(s->len, &tmp)) {
+ break;
+ }
+ if (!safeadd(tmp, &offset)) {
+ break;
+ }
s++;
} else {
- c = cut - offset;
+ c = cut;
+ if (!safesub(offset, &c)) {
+ break;
+ }
if (s->end < c)
c = s->end;
l = cut - offset - s->start;
@@ -165,8 +247,18 @@ static struct mpatch_flist *combine(struct mpatch_flis
/* insert new hunk */
ct = c->tail;
- ct->start = bh->start - offset;
- ct->end = bh->end - post;
+ ct->start = bh->start;
+ ct->end = bh->end;
+ if (!safesub(offset, &(ct->start)) ||
+ !safesub(post, &(ct->end))) {
+ /* It was already possible to exit
+ * this function with a return value
+ * of NULL before the safesub()s were
+ * added, so this should be fine. */
+ mpatch_lfree(c);
+ c = NULL;
+ goto done;
+ }
ct->len = bh->len;
ct->data = bh->data;
c->tail++;
@@ -177,7 +269,7 @@ static struct mpatch_flist *combine(struct mpatch_flis
memcpy(c->tail, a->head, sizeof(struct mpatch_frag) * lsize(a));
c->tail += lsize(a);
}
-
+done:
mpatch_lfree(a);
mpatch_lfree(b);
return c;
@@ -197,14 +289,21 @@ int mpatch_decode(const char *bin, ssize_t len, struct
lt = l->tail;
- while (pos >= 0 && pos < len) {
+ /* We check against len-11 to ensure we have at least 12 bytes
+ left in the patch so we can read our three be32s out of it. */
+ while (pos >= 0 && pos < (len - 11)) {
lt->start = getbe32(bin + pos);
lt->end = getbe32(bin + pos + 4);
lt->len = getbe32(bin + pos + 8);
- lt->data = bin + pos + 12;
- pos += 12 + lt->len;
- if (lt->start > lt->end || lt->len < 0)
+ if (lt->start < 0 || lt->start > lt->end || lt->len < 0)
break; /* sanity check */
+ if (!safeadd(12, &pos)) {
+ break;
+ }
+ lt->data = bin + pos;
+ if (!safeadd(lt->len, &pos)) {
+ break;
+ }
lt++;
}
@@ -246,7 +345,8 @@ int mpatch_apply(char *buf, const char *orig, ssize_t
char *p = buf;
while (f != l->tail) {
- if (f->start < last || f->end > len) {
+ if (f->start < last || f->start > len || f->end > len ||
+ last < 0) {
return MPATCH_ERR_INVALID_PATCH;
}
memcpy(p, orig + last, f->start - last);
@@ -255,6 +355,9 @@ int mpatch_apply(char *buf, const char *orig, ssize_t
last = f->end;
p += f->len;
f++;
+ }
+ if (last < 0) {
+ return MPATCH_ERR_INVALID_PATCH;
}
memcpy(p, orig + last, len - last);
return 0;