Attachment 15128 Details for Bug 13842 – git-am fix for master.

[patch] git-am fix for master.

bug-13842.master (text/plain), 11.09 KB, created by Jeremy Allison on 2019-05-07 19:33:24 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2019-05-07 19:33:24 UTC

Size: 11.09 KB

patch

obsolete

>From 2de28fb581c90fbb8bd820c8b19a5dc67df59065 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 25 Mar 2019 10:32:08 -0700
>Subject: [PATCH 1/3] s3: net: Harden guess_charset() against overflow errors.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/registry/reg_parse.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_parse.c b/source3/registry/reg_parse.c
>index c276c7e2d62..b15fde22fda 100644
>--- a/source3/registry/reg_parse.c
>+++ b/source3/registry/reg_parse.c
>@@ -671,7 +671,15 @@ static bool guess_charset(const char** ptr,
> 	}
> 
> 	if (srprs_bom(&pos, &charset, NULL)) {
>-		*len -= (pos - *ptr);
>+		size_t declen;
>+		if (pos < *ptr) {
>+			return false;
>+		}
>+		declen = (pos - *ptr);
>+		if (*len < declen) {
>+			return false;
>+		}
>+		*len -= declen;
> 		*ptr = pos;
> 		if (*file_enc == NULL) {
> 			*file_enc = charset;
>-- 
>2.21.0.1020.gf2820cf01a-goog
>
>
>From 133f437b0adcd800efe94e6feac7244c4ba39ec3 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 25 Mar 2019 11:13:24 -0700
>Subject: [PATCH 2/3] s3: net: Harden act_val_hex() act_val_sz() against
> errors.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/registry/reg_parse.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/source3/registry/reg_parse.c b/source3/registry/reg_parse.c
>index b15fde22fda..92642c5c0d6 100644
>--- a/source3/registry/reg_parse.c
>+++ b/source3/registry/reg_parse.c
>@@ -117,6 +117,7 @@ static bool act_val_hex(struct reg_parse* p, cbuf* value, bool cont)
> 					cbuf_swapptr(p->valblob, &dst, dlen);
> 				} else {
> 					DEBUG(0, ("iconvert_talloc failed\n"));
>+					return false;
> 				}
> 				talloc_free(dst);
> 			}
>@@ -166,6 +167,7 @@ static bool act_val_sz(struct reg_parse* p, cbuf* value, bool cont)
> 		} else {
> 			DEBUG(0, ("convert_string_talloc failed: >%s<\n"
> 				  "use it as is\t", src));
>+			return false;
> 		}
> 		talloc_free(dst);
> 
>-- 
>2.21.0.1020.gf2820cf01a-goog
>
>
>From 8af8264e4371c25e2fcf2b536fb3c184075c9481 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 7 May 2019 10:42:55 -0700
>Subject: [PATCH 3/3] s3: net: Rewrite of reg_parse_fd() to harden against
> buffer overwrites.
>
>Remove unused handle_iconv_errno().
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/registry/reg_parse.c | 292 +++++++++++++++++++++++------------
> 1 file changed, 193 insertions(+), 99 deletions(-)
>
>diff --git a/source3/registry/reg_parse.c b/source3/registry/reg_parse.c
>index 92642c5c0d6..575c7dc3409 100644
>--- a/source3/registry/reg_parse.c
>+++ b/source3/registry/reg_parse.c
>@@ -770,59 +770,13 @@ done:
> 	return ret;
> }
> 
>-
>-static void
>-handle_iconv_errno(int err, const char* obuf, size_t linenum,
>-		   smb_iconv_t cd, const char** iptr, size_t* ilen,
>-		   char** optr, size_t *olen)
>+static void display_iconv_error_bytes(const char *inbuf, size_t len)
> {
>-	const char *pos = obuf;
>-	const char *ptr = obuf;
>-	switch(err) {
>-	case EINVAL:
>-		/* DEBUG(0, ("Incomplete multibyte sequence\n")); */
>-	case E2BIG:
>-		return;
>-	case EILSEQ:
>-		break;
>-	default:
>-		assert(false);
>-	}
>-
>-	**optr = '\0';
>-	while (srprs_line(&ptr, NULL) && srprs_nl(&ptr, NULL)) {
>-		pos = ptr;
>-		linenum++;
>+	size_t i;
>+	for (i = 0; i < 4 && i < len; i++) {
>+		DEBUGADD(0, ("<%02x>", (unsigned char)inbuf[i]));
> 	}
>-	if (pos == *optr) {
>-		pos = MAX(obuf, *optr-60);
>-	}
>-	DEBUG(0, ("Illegal multibyte sequence at line %lu: %s",
>-		  (long unsigned)(linenum+1), pos));
>-
>-	assert((*ilen) > 0);
>-	do {
>-		size_t il = 1;
>-		DEBUGADD(0, ("<%02x>", (unsigned char)**iptr));
>-
>-		if ((*olen) > 0) {
>-			*(*optr)++ = '\?';
>-			(*iptr)++;
>-			/* Todo: parametrize, e.g. skip: *optr++ = *iptr++; */
>-			(*ilen)--;
>-		}
>-
>-		if (smb_iconv(cd, iptr, &il, optr, olen) != (size_t)-1 || (errno != EILSEQ)) {
>-			if(il == 0)
>-				(*ilen)-- ;
>-			break;
>-		}
>-
>-	}
>-	while ((*ilen > 0) && (*olen > 0));
>-
> 	DEBUGADD(0, ("\n"));
>-
> }
> 
> int reg_parse_fd(int fd, const struct reg_parse_callback* cb, const char* opts)
>@@ -831,105 +785,245 @@ int reg_parse_fd(int fd, const struct reg_parse_callback* cb, const char* opts)
> 	cbuf* line               = cbuf_new(mem_ctx);
> 	smb_iconv_t cd           = (smb_iconv_t)-1;
> 	struct reg_parse* parser = NULL;
>-	char buf_raw[1024];
>-	char buf_unix[1025];
>-
>+	char buf_in[1024];
>+	char buf_out[1025] = { 0 };
> 	ssize_t nread;
>-	size_t  nconv;
>-	const char* pos;
> 	const char* iptr;
> 	char* optr;
>         size_t ilen;
> 	size_t olen;
>+	size_t avail_osize = sizeof(buf_out)-1;
>+	size_t space_to_read = sizeof(buf_in);
> 	int ret = -1;
> 	bool eof = false;
>-	size_t linenum = 0;
>+	size_t linecount = 0;
> 
> 	struct reg_parse_fd_opt opt = reg_parse_fd_opt(mem_ctx, opts);
> 
> 	if (cb == NULL) {
>-		DEBUG(0,("reg_parse_fd: NULL callback\n"));
>+		DBG_ERR("NULL callback\n");
>+		ret = -1;
> 		goto done;
> 	}
> 
>-	nread = read(fd, buf_raw, sizeof(buf_raw));
>+	nread = read(fd, buf_in, space_to_read);
> 	if (nread < 0) {
>-		DEBUG(0, ("reg_parse_fd: read failed: %s\n", strerror(errno)));
>-		ret = nread;
>+		DBG_ERR("read failed: %s\n", strerror(errno));
>+		ret = -1;
>+		goto done;
>+	}
>+	if (nread == 0) {
>+		/* Empty file. */
>+		eof = true;
> 		goto done;
> 	}
> 
>-	iptr = &buf_raw[0];
>+	iptr = buf_in;
> 	ilen = nread;
> 
> 	if (!guess_charset(&iptr, &ilen,
> 			   &opt.file_enc, &opt.str_enc))
> 	{
>-		DEBUG(0, ("reg_parse_fd: failed to guess encoding\n"));
>+		DBG_ERR("reg_parse_fd: failed to guess encoding\n");
>+		ret = -1;
>+		goto done;
>+	}
>+
>+	if (ilen == 0) {
>+		/* File only contained charset info. */
>+		eof = true;
>+		ret = -1;
> 		goto done;
> 	}
> 
>-	DEBUG(10, ("reg_parse_fd: encoding file: %s str: %s\n",
>-		  opt.file_enc, opt.str_enc));
>+	DBG_DEBUG("reg_parse_fd: encoding file: %s str: %s\n",
>+		  opt.file_enc, opt.str_enc);
> 
> 
> 	if (!set_iconv(&cd, "unix", opt.file_enc)) {
>-		DEBUG(0, ("reg_parse_fd: failed to set file encoding %s\n",
>-			  opt.file_enc));
>+		DBG_ERR("reg_parse_fd: failed to set file encoding %s\n",
>+			  opt.file_enc);
>+		ret = -1;
> 		goto done;
> 	}
> 
> 	parser = reg_parse_new(mem_ctx, *cb, opt.str_enc, opt.flags);
> 
>-	optr = &buf_unix[0];
>+	/* Move input data to start of buf_in. */
>+	if (iptr > buf_in) {
>+		memmove(buf_in, iptr, ilen);
>+		iptr = buf_in;
>+	}
>+
>+	optr = buf_out;
>+	/* Leave last byte for null termination. */
>+	olen = avail_osize;
>+
>+	/*
>+	 * We read from buf_in (iptr), iconv converting into
>+	 * buf_out (optr).
>+	 */
>+
> 	while (!eof) {
>-		olen = sizeof(buf_unix) - (optr - buf_unix) - 1 ;
>-		while ( olen > 0 ) {
>-			memmove(buf_raw, iptr, ilen);
>-
>-			nread = read(fd, buf_raw + ilen, sizeof(buf_raw) - ilen);
>-			if (nread < 0) {
>-				DEBUG(0, ("reg_parse_fd: read failed: %s\n", strerror(errno)));
>-				ret = nread;
>+		const char *pos;
>+		size_t nconv;
>+
>+		if (olen == 0) {
>+			/* We're out of possible room. */
>+			DBG_ERR("no room in output buffer\n");
>+			ret = -1;
>+			goto done;
>+		}
>+		nconv = smb_iconv(cd, &iptr, &ilen, &optr, &olen);
>+		if (nconv == (size_t)-1) {
>+			bool valid_err = false;
>+			if (errno == EINVAL) {
>+				valid_err = true;
>+			}
>+			if (errno == E2BIG) {
>+				valid_err = true;
>+			}
>+			if (!valid_err) {
>+				DBG_ERR("smb_iconv error in file at line %zu: ",
>+					  linecount);
>+				display_iconv_error_bytes(iptr, ilen);
>+				ret = -1;
> 				goto done;
> 			}
>+			/*
>+			 * For valid errors process the
>+			 * existing buffer then continue.
>+			 */
>+		}
> 
>-			iptr =  buf_raw;
>-			ilen += nread;
>-
>-			if (ilen == 0) {
>-				smb_iconv(cd, NULL, NULL, &optr, &olen);
>-				eof = true;
>-				break;
>-			}
>+		/*
>+		 * We know this is safe as we have an extra
>+		 * enforced zero byte at the end of buf_out.
>+		 */
>+		*optr = '\0';
>+		pos = buf_out;
> 
>-			nconv = smb_iconv(cd, &iptr, &ilen, &optr, &olen);
>+		while (srprs_line(&pos, line) && srprs_nl_no_eos(&pos, line, eof)) {
>+			int retval;
> 
>-			if (nconv == (size_t)-1) {
>-				handle_iconv_errno(errno, buf_unix, linenum,
>-						   cd, &iptr, &ilen,
>-						   &optr, &olen);
>-				break;
>+			/* Process all lines we got. */
>+			linecount++;
>+			retval = reg_parse_line(parser, cbuf_gets(line, 0));
>+			if (retval < opt.fail_level) {
>+				DBG_ERR("reg_parse_line fail %d\n", retval);
>+				ret = -1;
>+				goto done;
> 			}
>+			cbuf_clear(line);
> 		}
>-	/* process_lines: */
>-		*optr = '\0';
>-		pos = &buf_unix[0];
>+		if (pos > buf_out) {
>+			/*
>+			 * The output data we have
>+			 * processed starts at buf_out
>+			 * and ends at pos.
>+			 * The unprocessed output
>+			 * data starts at pos and
>+			 * ends at optr.
>+			 *
>+			 *  <------ sizeof(buf_out) - 1------------->|0|
>+			 *  <--------- avail_osize------------------>|0|
>+			 *  +----------------------+-------+-----------+
>+			 *  |                      |       |         |0|
>+			 *  +----------------------+-------+-----------+
>+			 *  ^                      ^       ^
>+			 *  |                      |       |
>+			 * buf_out               pos      optr
>+			 */
>+			size_t unprocessed_len;
>+
>+			/* Paranoia checks. */
>+			if (optr < pos) {
>+				ret = -1;
>+				goto done;
>+			}
>+			unprocessed_len = optr - pos;
> 
>-		while ( srprs_line(&pos, line) && srprs_nl_no_eos(&pos, line, eof)) {
>-			linenum ++;
>-			ret = reg_parse_line(parser, cbuf_gets(line, 0));
>-			if (ret < opt.fail_level) {
>+			/* Paranoia checks. */
>+			if (avail_osize < unprocessed_len) {
>+				ret = -1;
> 				goto done;
> 			}
>-			cbuf_clear(line);
>+			/* Move down any unprocessed data. */
>+			memmove(buf_out, pos, unprocessed_len);
>+
>+			/*
>+			 * After the move, reset the output length.
>+			 *
>+			 *  <------ sizeof(buf_out) - 1------------->|0|
>+			 *  <--------- avail_osize------------------>|0|
>+			 *  +----------------------+-------+-----------+
>+			 *  |       |                                |0|
>+			 *  +----------------------+-------+-----------+
>+			 *  ^       ^
>+			 *  |       optr
>+			 * buf_out
>+			 */
>+			optr = buf_out + unprocessed_len;
>+			/*
>+			 * Calculate the new output space available
>+			 * for iconv.
>+			 * We already did the paranoia check for this
>+			 * arithmetic above.
>+			 */
>+			olen = avail_osize - unprocessed_len;
> 		}
>-		memmove(buf_unix, pos, optr - pos);
>-		optr -= (pos - buf_unix);
>+
>+		/*
>+		 * Move any unprocessed data to the start of
>+		 * the input buffer (buf_in).
>+		 */
>+		if (ilen > 0 && iptr > buf_in) {
>+			memmove(buf_in, iptr, ilen);
>+		}
>+
>+		/* Is there any space to read more input ? */
>+		if (ilen >= sizeof(buf_in)) {
>+			/* No space. Nothing was converted. Error. */
>+			DBG_ERR("no space in input buffer\n");
>+			ret = -1;
>+			goto done;
>+		}
>+
>+		space_to_read = sizeof(buf_in) - ilen;
>+
>+		/* Read the next chunk from the file. */
>+		nread = read(fd, buf_in, space_to_read);
>+		if (nread < 0) {
>+			DBG_ERR("read failed: %s\n", strerror(errno));
>+			ret = -1;
>+			goto done;
>+		}
>+		if (nread == 0) {
>+			/* Empty file. */
>+			eof = true;
>+			continue;
>+		}
>+
>+		/* Paranoia check. */
>+		if (nread + ilen < ilen) {
>+			ret = -1;
>+			goto done;
>+		}
>+
>+		/* Paranoia check. */
>+		if (nread + ilen > sizeof(buf_in)) {
>+			ret = -1;
>+			goto done;
>+		}
>+
>+		iptr = buf_in;
>+		ilen = nread + ilen;
> 	}
> 
> 	ret = 0;
>+
> done:
>+
> 	set_iconv(&cd, NULL, NULL);
> 	talloc_free(mem_ctx);
> 	return ret;
>-- 
>2.21.0.1020.gf2820cf01a-goog
>

Actions: View

Attachments on bug 13842: 15128 | 15142 | 15146 | 15148