|
|
cf659d |
From 8e9016a11c7ebd08e92277962e495945a3ad588f Mon Sep 17 00:00:00 2001
|
|
|
cf659d |
From: Jeremy Allison <jra@samba.org>
|
|
|
cf659d |
Date: Fri, 15 Jun 2018 15:07:17 -0700
|
|
|
cf659d |
Subject: [PATCH 1/2] libsmb: Ensure smbc_urlencode() can't overwrite passed in
|
|
|
cf659d |
buffer.
|
|
|
cf659d |
|
|
|
cf659d |
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13453
|
|
|
cf659d |
|
|
|
cf659d |
CVE-2018-10858: Insufficient input validation on client directory
|
|
|
cf659d |
listing in libsmbclient.
|
|
|
cf659d |
|
|
|
cf659d |
Signed-off-by: Jeremy Allison <jra@samba.org>
|
|
|
cf659d |
Reviewed-by: Ralph Boehme <slow@samba.org>
|
|
|
cf659d |
---
|
|
|
cf659d |
source3/libsmb/libsmb_path.c | 9 +++++++--
|
|
|
cf659d |
1 file changed, 7 insertions(+), 2 deletions(-)
|
|
|
cf659d |
|
|
|
cf659d |
diff --git a/source3/libsmb/libsmb_path.c b/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
index 01b0a61e483..ed70ab37550 100644
|
|
|
cf659d |
--- a/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
+++ b/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
@@ -173,8 +173,13 @@ smbc_urlencode(char *dest,
|
|
|
cf659d |
}
|
|
|
cf659d |
}
|
|
|
cf659d |
|
|
|
cf659d |
- *dest++ = '\0';
|
|
|
cf659d |
- max_dest_len--;
|
|
|
cf659d |
+ if (max_dest_len == 0) {
|
|
|
cf659d |
+ /* Ensure we return -1 if no null termination. */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+
|
|
|
cf659d |
+ *dest++ = '\0';
|
|
|
cf659d |
+ max_dest_len--;
|
|
|
cf659d |
|
|
|
cf659d |
return max_dest_len;
|
|
|
cf659d |
}
|
|
|
cf659d |
--
|
|
|
cf659d |
2.11.0
|
|
|
cf659d |
|
|
|
cf659d |
|
|
|
cf659d |
From 0a259d3c56b7e436c0b589b175619565e0515fa0 Mon Sep 17 00:00:00 2001
|
|
|
cf659d |
From: Jeremy Allison <jra@samba.org>
|
|
|
cf659d |
Date: Fri, 15 Jun 2018 15:08:17 -0700
|
|
|
cf659d |
Subject: [PATCH 2/2] libsmb: Harden smbc_readdir_internal() against returns
|
|
|
cf659d |
from malicious servers.
|
|
|
cf659d |
|
|
|
cf659d |
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13453
|
|
|
cf659d |
|
|
|
cf659d |
CVE-2018-10858: Insufficient input validation on client directory
|
|
|
cf659d |
listing in libsmbclient.
|
|
|
cf659d |
|
|
|
cf659d |
Signed-off-by: Jeremy Allison <jra@samba.org>
|
|
|
cf659d |
Reviewed-by: Ralph Boehme <slow@samba.org>
|
|
|
cf659d |
---
|
|
|
cf659d |
source3/libsmb/libsmb_dir.c | 57 ++++++++++++++++++++++++++++++++++++++------
|
|
|
cf659d |
source3/libsmb/libsmb_path.c | 2 +-
|
|
|
cf659d |
2 files changed, 51 insertions(+), 8 deletions(-)
|
|
|
cf659d |
|
|
|
cf659d |
diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
|
|
|
cf659d |
index 72441c46736..54c2bcb3c73 100644
|
|
|
cf659d |
--- a/source3/libsmb/libsmb_dir.c
|
|
|
cf659d |
+++ b/source3/libsmb/libsmb_dir.c
|
|
|
cf659d |
@@ -943,27 +943,47 @@ SMBC_closedir_ctx(SMBCCTX *context,
|
|
|
cf659d |
|
|
|
cf659d |
}
|
|
|
cf659d |
|
|
|
cf659d |
-static void
|
|
|
cf659d |
+static int
|
|
|
cf659d |
smbc_readdir_internal(SMBCCTX * context,
|
|
|
cf659d |
struct smbc_dirent *dest,
|
|
|
cf659d |
struct smbc_dirent *src,
|
|
|
cf659d |
int max_namebuf_len)
|
|
|
cf659d |
{
|
|
|
cf659d |
if (smbc_getOptionUrlEncodeReaddirEntries(context)) {
|
|
|
cf659d |
+ int remaining_len;
|
|
|
cf659d |
|
|
|
cf659d |
/* url-encode the name. get back remaining buffer space */
|
|
|
cf659d |
- max_namebuf_len =
|
|
|
cf659d |
+ remaining_len =
|
|
|
cf659d |
smbc_urlencode(dest->name, src->name, max_namebuf_len);
|
|
|
cf659d |
|
|
|
cf659d |
+ /* -1 means no null termination. */
|
|
|
cf659d |
+ if (remaining_len < 0) {
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+
|
|
|
cf659d |
/* We now know the name length */
|
|
|
cf659d |
dest->namelen = strlen(dest->name);
|
|
|
cf659d |
|
|
|
cf659d |
+ if (dest->namelen + 1 < 1) {
|
|
|
cf659d |
+ /* Integer wrap. */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+
|
|
|
cf659d |
+ if (dest->namelen + 1 >= max_namebuf_len) {
|
|
|
cf659d |
+ /* Out of space for comment. */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+
|
|
|
cf659d |
/* Save the pointer to the beginning of the comment */
|
|
|
cf659d |
dest->comment = dest->name + dest->namelen + 1;
|
|
|
cf659d |
|
|
|
cf659d |
+ if (remaining_len < 1) {
|
|
|
cf659d |
+ /* No room for comment null termination. */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+
|
|
|
cf659d |
/* Copy the comment */
|
|
|
cf659d |
- strncpy(dest->comment, src->comment, max_namebuf_len - 1);
|
|
|
cf659d |
- dest->comment[max_namebuf_len - 1] = '\0';
|
|
|
cf659d |
+ strlcpy(dest->comment, src->comment, remaining_len);
|
|
|
cf659d |
|
|
|
cf659d |
/* Save other fields */
|
|
|
cf659d |
dest->smbc_type = src->smbc_type;
|
|
|
cf659d |
@@ -973,10 +993,21 @@ smbc_readdir_internal(SMBCCTX * context,
|
|
|
cf659d |
} else {
|
|
|
cf659d |
|
|
|
cf659d |
/* No encoding. Just copy the entry as is. */
|
|
|
cf659d |
+ if (src->dirlen > max_namebuf_len) {
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
memcpy(dest, src, src->dirlen);
|
|
|
cf659d |
+ if (src->namelen + 1 < 1) {
|
|
|
cf659d |
+ /* Integer wrap */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
+ if (src->namelen + 1 >= max_namebuf_len) {
|
|
|
cf659d |
+ /* Comment off the end. */
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
dest->comment = (char *)(&dest->name + src->namelen + 1);
|
|
|
cf659d |
}
|
|
|
cf659d |
-
|
|
|
cf659d |
+ return 0;
|
|
|
cf659d |
}
|
|
|
cf659d |
|
|
|
cf659d |
/*
|
|
|
cf659d |
@@ -988,6 +1019,7 @@ SMBC_readdir_ctx(SMBCCTX *context,
|
|
|
cf659d |
SMBCFILE *dir)
|
|
|
cf659d |
{
|
|
|
cf659d |
int maxlen;
|
|
|
cf659d |
+ int ret;
|
|
|
cf659d |
struct smbc_dirent *dirp, *dirent;
|
|
|
cf659d |
TALLOC_CTX *frame = talloc_stackframe();
|
|
|
cf659d |
|
|
|
cf659d |
@@ -1037,7 +1069,12 @@ SMBC_readdir_ctx(SMBCCTX *context,
|
|
|
cf659d |
dirp = &context->internal->dirent;
|
|
|
cf659d |
maxlen = sizeof(context->internal->_dirent_name);
|
|
|
cf659d |
|
|
|
cf659d |
- smbc_readdir_internal(context, dirp, dirent, maxlen);
|
|
|
cf659d |
+ ret = smbc_readdir_internal(context, dirp, dirent, maxlen);
|
|
|
cf659d |
+ if (ret == -1) {
|
|
|
cf659d |
+ errno = EINVAL;
|
|
|
cf659d |
+ TALLOC_FREE(frame);
|
|
|
cf659d |
+ return NULL;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
|
|
|
cf659d |
dir->dir_next = dir->dir_next->next;
|
|
|
cf659d |
|
|
|
cf659d |
@@ -1095,6 +1132,7 @@ SMBC_getdents_ctx(SMBCCTX *context,
|
|
|
cf659d |
*/
|
|
|
cf659d |
|
|
|
cf659d |
while ((dirlist = dir->dir_next)) {
|
|
|
cf659d |
+ int ret;
|
|
|
cf659d |
struct smbc_dirent *dirent;
|
|
|
cf659d |
struct smbc_dirent *currentEntry = (struct smbc_dirent *)ndir;
|
|
|
cf659d |
|
|
|
cf659d |
@@ -1109,8 +1147,13 @@ SMBC_getdents_ctx(SMBCCTX *context,
|
|
|
cf659d |
/* Do urlencoding of next entry, if so selected */
|
|
|
cf659d |
dirent = &context->internal->dirent;
|
|
|
cf659d |
maxlen = sizeof(context->internal->_dirent_name);
|
|
|
cf659d |
- smbc_readdir_internal(context, dirent,
|
|
|
cf659d |
+ ret = smbc_readdir_internal(context, dirent,
|
|
|
cf659d |
dirlist->dirent, maxlen);
|
|
|
cf659d |
+ if (ret == -1) {
|
|
|
cf659d |
+ errno = EINVAL;
|
|
|
cf659d |
+ TALLOC_FREE(frame);
|
|
|
cf659d |
+ return -1;
|
|
|
cf659d |
+ }
|
|
|
cf659d |
|
|
|
cf659d |
reqd = dirent->dirlen;
|
|
|
cf659d |
|
|
|
cf659d |
diff --git a/source3/libsmb/libsmb_path.c b/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
index ed70ab37550..5b53b386a67 100644
|
|
|
cf659d |
--- a/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
+++ b/source3/libsmb/libsmb_path.c
|
|
|
cf659d |
@@ -173,7 +173,7 @@ smbc_urlencode(char *dest,
|
|
|
cf659d |
}
|
|
|
cf659d |
}
|
|
|
cf659d |
|
|
|
cf659d |
- if (max_dest_len == 0) {
|
|
|
cf659d |
+ if (max_dest_len <= 0) {
|
|
|
cf659d |
/* Ensure we return -1 if no null termination. */
|
|
|
cf659d |
return -1;
|
|
|
cf659d |
}
|
|
|
cf659d |
--
|
|
|
cf659d |
2.11.0
|
|
|
cf659d |
|