The branch, master has been updated
via f0ea0800982 s3: net: Test of fuzzer problems with net rpc registry
import.
via 11c35c8f783 s3: net: Rewrite of reg_parse_fd() to harden against
buffer overwrites.
via 70025b4a703 s3: net: Harden srprs_str() against memcmp overread.
via b3bfad39d64 s3: net: Harden act_val_hex() act_val_sz() against
errors.
via 226544f6f56 s3: net: Harden guess_charset() against overflow errors.
from 0daa0ff921b s4 dsdb/repl_meta_data: fix use after free in
dsdb_audit_add_ldb_value
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit f0ea08009821805b1abfe2ff3b2a3d5ee96de396
Author: Jeremy Allison <[email protected]>
Date: Thu May 9 14:34:37 2019 -0700
s3: net: Test of fuzzer problems with net rpc registry import.
Found by Michael Hanselmann using fuzzing tools
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
Autobuild-User(master): Andrew Bartlett <[email protected]>
Autobuild-Date(master): Wed May 15 23:08:58 UTC 2019 on sn-devel-184
commit 11c35c8f783d359fcc3ff6f22d19aae5836a16d2
Author: Jeremy Allison <[email protected]>
Date: Tue May 7 10:42:55 2019 -0700
s3: net: Rewrite of reg_parse_fd() to harden against buffer overwrites.
Remove unused handle_iconv_errno(). Fix leaks of iconv handles.
Found by Michael Hanselmann using fuzzing tools
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
commit 70025b4a70364a68d08e2880675449e4e4729420
Author: Jeremy Allison <[email protected]>
Date: Mon May 13 13:45:10 2019 -0700
s3: net: Harden srprs_str() against memcmp overread.
Found by Michael Hanselmann using fuzzing tools
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
commit b3bfad39d64eab0e3a9218182b34385c2a397ff5
Author: Jeremy Allison <[email protected]>
Date: Mon Mar 25 11:13:24 2019 -0700
s3: net: Harden act_val_hex() act_val_sz() against errors.
Found by Michael Hanselmann using fuzzing tools
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
commit 226544f6f5699891bbd933361c65750a26cfaccf
Author: Jeremy Allison <[email protected]>
Date: Mon Mar 25 10:32:08 2019 -0700
s3: net: Harden guess_charset() against overflow errors.
Found by Michael Hanselmann using fuzzing tools
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13842
Signed-off-by: Jeremy Allison <[email protected]>
Reviewed-by: Andrew Bartlett <[email protected]>
-----------------------------------------------------------------------
Summary of changes:
source3/lib/srprs.c | 8 +
source3/registry/reg_parse.c | 314 +++++++++++++++--------
source3/script/tests/test_net_registry_import.sh | 192 ++++++++++++++
source3/selftest/tests.py | 1 +
4 files changed, 415 insertions(+), 100 deletions(-)
create mode 100755 source3/script/tests/test_net_registry_import.sh
Changeset truncated at 500 lines:
diff --git a/source3/lib/srprs.c b/source3/lib/srprs.c
index 02f4c80e27b..67ada3796f0 100644
--- a/source3/lib/srprs.c
+++ b/source3/lib/srprs.c
@@ -46,9 +46,17 @@ bool srprs_char(const char** ptr, char c) {
bool srprs_str(const char** ptr, const char* str, ssize_t len)
{
+ /* By definition *ptr must be null terminated. */
+ size_t ptr_len = strlen(*ptr);
+
if (len == -1)
len = strlen(str);
+ /* Don't memcmp read past end of buffer. */
+ if (len > ptr_len) {
+ return false;
+ }
+
if (memcmp(*ptr, str, len) == 0) {
*ptr += len;
return true;
diff --git a/source3/registry/reg_parse.c b/source3/registry/reg_parse.c
index 81815a4fd98..c64cf66a5ab 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);
@@ -263,6 +265,7 @@ struct reg_parse* reg_parse_new(const void* ctx,
assert(&s->reg_format_callback == (struct reg_format_callback*)s);
return s;
fail:
+ set_iconv(&s->str2UTF16, NULL, NULL);
talloc_free(s);
return NULL;
}
@@ -688,7 +691,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;
@@ -777,59 +788,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++;
- }
- 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;
- }
-
+ size_t i;
+ for (i = 0; i < 4 && i < len; i++) {
+ DEBUGADD(0, ("<%02x>", (unsigned char)inbuf[i]));
}
- while ((*ilen > 0) && (*olen > 0));
-
DEBUGADD(0, ("\n"));
-
}
int reg_parse_fd(int fd, const struct reg_parse_callback* cb, const char* opts)
@@ -838,106 +803,255 @@ 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;
}
- DEBUG(10, ("reg_parse_fd: encoding file: %s str: %s\n",
- opt.file_enc, opt.str_enc));
+ if (ilen == 0) {
+ /* File only contained charset info. */
+ eof = true;
+ ret = -1;
+ goto done;
+ }
+
+ 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);
+ if (parser == NULL) {
+ ret = -1;
+ goto done;
+ }
+
+ /* 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).
+ */
- optr = &buf_unix[0];
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. */
+ retval = reg_parse_line(parser, cbuf_gets(line, 0));
+ if (retval < opt.fail_level) {
+ DBG_ERR("reg_parse_line %zu fail %d\n",
+ linecount,
+ retval);
+ ret = -1;
+ goto done;
}
+ cbuf_clear(line);
+ linecount++;
}
- /* 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);
+ if (parser) {
+ set_iconv(&parser->str2UTF16, NULL, NULL);
+ }
talloc_free(mem_ctx);
return ret;
}
diff --git a/source3/script/tests/test_net_registry_import.sh
b/source3/script/tests/test_net_registry_import.sh
new file mode 100755
index 00000000000..40e68fad875
--- /dev/null
+++ b/source3/script/tests/test_net_registry_import.sh
@@ -0,0 +1,192 @@
+#!/bin/sh
+
+if [ $# -lt 4 ]; then
+cat <<EOF
+Usage: test_net_registry_import.sh SERVER LOCAL_PATH USERNAME PASSWORD
+EOF
+exit 1;
+fi
+
+SERVER="$1"
+LOCAL_PATH="$2"
+USERNAME="$3"
+PASSWORD="$4"
+shift 4
+ADDARGS="$@"
+
+failed=0
+
+samba_net="$BINDIR/net"
+
+incdir=`dirname $0`/../../../testprogs/blackbox
+. $incdir/subunit.sh
+
+
+test_net_registry_import() {
+
+#
+# Expect:
+# Found Byte Order Mark for : UTF-16LE
+#
+ cmd='$VALGRIND $samba_net rpc registry import
$LOCAL_PATH/case3b45ccc3b.dat -S$SERVER -U$USERNAME%$PASSWORD $ADDARGS'
+
+ eval echo "$cmd"
+ out=`eval $cmd 2>&1`
+ ret=$?
+
+ if [ $ret != 0 ] ; then
+ echo "$out"
+ echo "command failed with output $ret"
+ false
+ return
+ fi
+
+ echo "$out" | grep 'Found Byte Order Mark for : UTF-16LE'
+ ret=$?
+
+ if [ $ret -ne 0 ] ; then
+ echo "$out"
+ echo "$samba_net rpc registry import
$LOCAL_PATH/case3b45ccc3b.dat failed - should get 'Found Byte Order Mark for :
UTF-16LE'"
+ false
+ return
+ fi
+
+#
+# Expect:
+# reg_parse_fd: smb_iconv error in file at line 0: <bf><77><d4><41>
+#
+ cmd='$VALGRIND $samba_net rpc registry import
$LOCAL_PATH/casecbe8c2427.dat -S$SERVER -U$USERNAME%$PASSWORD $ADDARGS'
--
Samba Shared Repository
