summaryrefslogtreecommitdiff
path: root/source3
diff options
context:
space:
mode:
authorJeremy Allison <jra@samba.org>2018-06-15 15:08:17 -0700
committerKarolin Seeger <kseeger@samba.org>2018-08-14 13:57:16 +0200
commitbe3d4b2d8526add6a487c9977de5a42704895833 (patch)
tree48a754f3d7d04f41adc6300deb29006a87bd8417 /source3
parent860f575fa5987413b5d5efa9a218a4cc3bf2edb9 (diff)
downloadsamba-be3d4b2d8526add6a487c9977de5a42704895833.tar.gz
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>
Diffstat (limited to 'source3')
-rw-r--r--source3/libsmb/libsmb_dir.c57
-rw-r--r--source3/libsmb/libsmb_path.c2
2 files changed, 51 insertions, 8 deletions
diff --git a/source3/libsmb/libsmb_dir.c b/source3/libsmb/libsmb_dir.c
index 41744eaef81..ff19ed57f14 100644
--- a/source3/libsmb/libsmb_dir.c
+++ b/source3/libsmb/libsmb_dir.c
@@ -1038,27 +1038,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;
@@ -1068,10 +1088,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;
}
/*
@@ -1083,6 +1114,7 @@ SMBC_readdir_ctx(SMBCCTX *context,
SMBCFILE *dir)
{
int maxlen;
+ int ret;
struct smbc_dirent *dirp, *dirent;
TALLOC_CTX *frame = talloc_stackframe();
@@ -1132,7 +1164,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;
@@ -1236,6 +1273,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;
@@ -1250,8 +1288,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;
}