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