CVE-2019-10218 - s3: libsmb: Protect SMB1 client code from evil server returned names.

Disconnect with NT_STATUS_INVALID_NETWORK_RESPONSE if so.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14071

Signed-off-by: Jeremy Allison <jra@samba.org>

diff --git a/source3/libsmb/clilist.c b/source3/libsmb/clilist.c
index 58dac65f4c73cf08628cb5b3e40b82635f3de890..f868e72a239e4b2aa2ba230fa08b80c834899d37 100644 (file)
--- a/source3/libsmb/clilist.c
+++ b/source3/libsmb/clilist.c
@@ -24,6 +24,66 @@
 #include "trans2.h"
 #include "../libcli/smb/smbXcli_base.h"
 
+/****************************************************************************
+ Check if a returned directory name is safe.
+****************************************************************************/
+
+static NTSTATUS is_bad_name(bool windows_names, const char *name)
+{
+       const char *bad_name_p = NULL;
+
+       bad_name_p = strchr(name, '/');
+       if (bad_name_p != NULL) {
+               /*
+                * Windows and POSIX names can't have '/'.
+                * Server is attacking us.
+                */
+               return NT_STATUS_INVALID_NETWORK_RESPONSE;
+       }
+       if (windows_names) {
+               bad_name_p = strchr(name, '\\');
+               if (bad_name_p != NULL) {
+                       /*
+                        * Windows names can't have '\\'.
+                        * Server is attacking us.
+                        */
+                       return NT_STATUS_INVALID_NETWORK_RESPONSE;
+               }
+       }
+       return NT_STATUS_OK;
+}
+
+/****************************************************************************
+ Check if a returned directory name is safe. Disconnect if server is
+ sending bad names.
+****************************************************************************/
+
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+                       const struct file_info *finfo)
+{
+       NTSTATUS status = NT_STATUS_OK;
+       bool windows_names = true;
+
+       if (cli->requested_posix_capabilities & CIFS_UNIX_POSIX_PATHNAMES_CAP) {
+               windows_names = false;
+       }
+       if (finfo->name != NULL) {
+               status = is_bad_name(windows_names, finfo->name);
+               if (!NT_STATUS_IS_OK(status)) {
+                       DBG_ERR("bad finfo->name\n");
+                       return status;
+               }
+       }
+       if (finfo->short_name != NULL) {
+               status = is_bad_name(windows_names, finfo->short_name);
+               if (!NT_STATUS_IS_OK(status)) {
+                       DBG_ERR("bad finfo->short_name\n");
+                       return status;
+               }
+       }
+       return NT_STATUS_OK;
+}
+
 /****************************************************************************
  Calculate a safe next_entry_offset.
 ****************************************************************************/
@@ -492,6 +552,13 @@ static NTSTATUS cli_list_old_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
                        TALLOC_FREE(finfo);
                        return NT_STATUS_NO_MEMORY;
                }
+
+               status = is_bad_finfo_name(state->cli, finfo);
+               if (!NT_STATUS_IS_OK(status)) {
+                       smbXcli_conn_disconnect(state->cli->conn, status);
+                       TALLOC_FREE(finfo);
+                       return status;
+               }
        }
        *pfinfo = finfo;
        return NT_STATUS_OK;
@@ -727,6 +794,14 @@ static void cli_list_trans_done(struct tevent_req *subreq)
                        ff_eos = true;
                        break;
                }
+
+               status = is_bad_finfo_name(state->cli, finfo);
+               if (!NT_STATUS_IS_OK(status)) {
+                       smbXcli_conn_disconnect(state->cli->conn, status);
+                       tevent_req_nterror(req, status);
+                       return;
+               }
+
                if (!state->first && (state->mask[0] != '\0') &&
                    strcsequal(finfo->name, state->mask)) {
                        DEBUG(1, ("Error: Looping in FIND_NEXT as name %s has "

diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 6a647da58c825d5ec8be02035066ede8b7bc5c39..48855d7112cf4ea6227ed9e475f080041638e562 100644 (file)
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -760,6 +760,9 @@ NTSTATUS cli_posix_whoami(struct cli_state *cli,
 
 /* The following definitions come from libsmb/clilist.c  */
 
+NTSTATUS is_bad_finfo_name(const struct cli_state *cli,
+                       const struct file_info *finfo);
+
 NTSTATUS cli_list_old(struct cli_state *cli,const char *Mask,uint16_t attribute,
                      NTSTATUS (*fn)(const char *, struct file_info *,
                                 const char *, void *), void *state);