src/patches/samba/samba-3.6.99-fix_symlink_verification.patch

   1 From b6192b3cdeaa9eb719ec5da3977af9470504d294 Mon Sep 17 00:00:00 2001
   2 From: Michael Adam <obnox@samba.org>
   3 Date: Wed, 23 Dec 2015 18:01:23 +0100
   4 Subject: [PATCH] s3:smbd: fix a corner case of the symlink verification
   5
   6 Commit 7606c0db257b3f9d84da5b2bf5fbb4034cc8d77d fixes the
   7 path checks in check_reduced_name[_with_privilege]() to
   8 prevent unintended access via wide links.
   9
  10 The fix fails to correctly treat a corner case where the share
  11 path is "/". This case is important for some real world
  12 scenarios, notably the use of the glusterfs VFS module:
  13
  14 For the share path "/", the newly introduced checks deny all
  15 operations in the share.
  16
  17 This change fixes the checks for the corner case.
  18 The point is that the assumptions on which the original
  19 checks are based are not true for the rootdir "/" case.
  20 This is the case where the rootdir starts _and ends_ with
  21 a slash. Hence a subdirectory does not continue with a
  22 slash after the rootdir, since the candidate path has
  23 been normalized.
  24
  25 This fix just omits the string comparison and the
  26 next character checks in the case of rootdir "/",
  27 which is correct because we know that the candidate
  28 path is normalized and hence starts with a '/'.
  29
  30 The patch is fairly minimal, but changes indentation,
  31 hence best viewed with 'git show -w'.
  32
  33 A side effect is that the rootdir="/" case needs
  34 one strncmp less.
  35
  36 BUG: https://bugzilla.samba.org/show_bug.cgi?id=11647
  37
  38 Pair-Programmed-With: Jose A. Rivera <jarrpa@samba.org>
  39
  40 Signed-off-by: Michael Adam <obnox@samba.org>
  41 Signed-off-by: Jose A. Rivera <jarrpa@samba.org>
  42 Reviewed-by: Jeremy Allison <jra@samba.org>
  43
  44 Autobuild-User(master): Michael Adam <obnox@samba.org>
  45 Autobuild-Date(master): Thu Dec 24 00:57:31 CET 2015 on sn-devel-144
  46
  47 (cherry picked from commit ada59ec7b3a5ed0478d11da2fe0c90991d137288)
  48 ---
  49  source3/smbd/vfs.c | 39 +++++++++++++++++++++++++++------------
  50  1 file changed, 27 insertions(+), 12 deletions(-)
  51
  52 diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
  53 index bd93b7f..2b8000d 100644
  54 --- a/source3/smbd/vfs.c
  55 +++ b/source3/smbd/vfs.c
  56 @@ -982,7 +982,6 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
  57         if (!allow_widelinks || !allow_symlinks) {
  58                 const char *conn_rootdir;
  59                 size_t rootdir_len;
  60 -               bool matched;
  61
  62                 conn_rootdir = SMB_VFS_CONNECTPATH(conn, fname);
  63                 if (conn_rootdir == NULL) {
  64 @@ -993,17 +992,33 @@ NTSTATUS check_reduced_name(connection_struct *conn, const char *fname)
  65                 }
  66
  67                 rootdir_len = strlen(conn_rootdir);
  68 -               matched = (strncmp(conn_rootdir, resolved_name,
  69 -                               rootdir_len) == 0);
  70 -               if (!matched || (resolved_name[rootdir_len] != '/' &&
  71 -                                resolved_name[rootdir_len] != '\0')) {
  72 -                       DEBUG(2, ("check_reduced_name: Bad access "
  73 -                               "attempt: %s is a symlink outside the "
  74 -                               "share path\n", fname));
  75 -                       DEBUGADD(2, ("conn_rootdir =%s\n", conn_rootdir));
  76 -                       DEBUGADD(2, ("resolved_name=%s\n", resolved_name));
  77 -                       SAFE_FREE(resolved_name);
  78 -                       return NT_STATUS_ACCESS_DENIED;
  79 +
  80 +               /*
  81 +                * In the case of rootdir_len == 1, we know that
  82 +                * conn_rootdir is "/", and we also know that
  83 +                * resolved_name starts with a slash.  So, in this
  84 +                * corner case, resolved_name is automatically a
  85 +                * sub-directory of the conn_rootdir. Thus we can skip
  86 +                * the string comparison and the next character checks
  87 +                * (which are even wrong in this case).
  88 +                */
  89 +               if (rootdir_len != 1) {
  90 +                       bool matched;
  91 +
  92 +                       matched = (strncmp(conn_rootdir, resolved_name,
  93 +                                       rootdir_len) == 0);
  94 +                       if (!matched || (resolved_name[rootdir_len] != '/' &&
  95 +                                        resolved_name[rootdir_len] != '\0')) {
  96 +                               DEBUG(2, ("check_reduced_name: Bad access "
  97 +                                       "attempt: %s is a symlink outside the "
  98 +                                       "share path\n", fname));
  99 +                               DEBUGADD(2, ("conn_rootdir =%s\n",
 100 +                                            conn_rootdir));
 101 +                               DEBUGADD(2, ("resolved_name=%s\n",
 102 +                                            resolved_name));
 103 +                               SAFE_FREE(resolved_name);
 104 +                               return NT_STATUS_ACCESS_DENIED;
 105 +                       }
 106                 }
 107
 108                 /* Extra checks if all symlinks are disallowed. */
 109 --
 110 2.5.0
 111