|
 |
887953 |
From fe704e0f997444d74966aa7c5bfca484ce54f6a4 Mon Sep 17 00:00:00 2001
|
|
|
887953 |
From: Amar Tumballi <amarts@redhat.com>
|
|
|
887953 |
Date: Thu, 27 Sep 2018 09:55:19 +0530
|
|
|
887953 |
Subject: [PATCH 389/399] server: don't allow '/' in basename
|
|
|
887953 |
|
|
|
887953 |
Server stack needs to have all the sort of validation, assuming
|
|
|
887953 |
clients can be compromized. It is possible for a compromized
|
|
|
887953 |
client to send basenames with paths with '/', and with that
|
|
|
887953 |
create files without permission on server. By sanitizing the basename,
|
|
|
887953 |
and not allowing anything other than actual directory as the parent
|
|
|
887953 |
for any entry creation, we can mitigate the effects of clients
|
|
|
887953 |
not able to exploit the server.
|
|
|
887953 |
|
|
|
887953 |
Fixes: CVE-2018-14651
|
|
|
887953 |
|
|
|
887953 |
BUG: 1633013
|
|
|
887953 |
Change-Id: I98d042a9f8e300161fbc3ee5b6e8de755c9765f9
|
|
|
887953 |
Signed-off-by: Amar Tumballi <amarts@redhat.com>
|
|
|
887953 |
Reviewed-on: https://code.engineering.redhat.com/gerrit/151169
|
|
|
887953 |
Reviewed-by: Xavi Hernandez <xhernandez@redhat.com>
|
|
|
887953 |
Reviewed-by: Shyam Ranganathan <srangana@redhat.com>
|
|
|
887953 |
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
|
|
|
887953 |
---
|
|
|
887953 |
xlators/protocol/server/src/server-resolve.c | 31 ++++++++++++++++++++--------
|
|
|
887953 |
xlators/storage/posix/src/posix-handle.h | 5 +++--
|
|
|
887953 |
2 files changed, 25 insertions(+), 11 deletions(-)
|
|
|
887953 |
|
|
|
887953 |
diff --git a/xlators/protocol/server/src/server-resolve.c b/xlators/protocol/server/src/server-resolve.c
|
|
|
887953 |
index b3eda0e..25db43f 100644
|
|
|
887953 |
--- a/xlators/protocol/server/src/server-resolve.c
|
|
|
887953 |
+++ b/xlators/protocol/server/src/server-resolve.c
|
|
|
887953 |
@@ -307,22 +307,35 @@ resolve_entry_simple (call_frame_t *frame)
|
|
|
887953 |
ret = 1;
|
|
|
887953 |
goto out;
|
|
|
887953 |
}
|
|
|
887953 |
-
|
|
|
887953 |
- /* expected @parent was found from the inode cache */
|
|
|
887953 |
- gf_uuid_copy (state->loc_now->pargfid, resolve->pargfid);
|
|
|
887953 |
- state->loc_now->parent = inode_ref (parent);
|
|
|
887953 |
-
|
|
|
887953 |
- if (strstr (resolve->bname, "../")) {
|
|
|
887953 |
- /* Resolving outside the parent's tree is not allowed */
|
|
|
887953 |
+ if (parent->ia_type != IA_IFDIR) {
|
|
|
887953 |
+ /* Parent type should be 'directory', and nothing else */
|
|
|
887953 |
gf_msg (this->name, GF_LOG_ERROR, EPERM,
|
|
|
887953 |
PS_MSG_GFID_RESOLVE_FAILED,
|
|
|
887953 |
- "%s: path sent by client not allowed",
|
|
|
887953 |
- resolve->bname);
|
|
|
887953 |
+ "%s: parent type not directory (%d)",
|
|
|
887953 |
+ uuid_utoa (parent->gfid), parent->ia_type);
|
|
|
887953 |
resolve->op_ret = -1;
|
|
|
887953 |
resolve->op_errno = EPERM;
|
|
|
887953 |
ret = 1;
|
|
|
887953 |
goto out;
|
|
|
887953 |
}
|
|
|
887953 |
+
|
|
|
887953 |
+ /* expected @parent was found from the inode cache */
|
|
|
887953 |
+ gf_uuid_copy (state->loc_now->pargfid, resolve->pargfid);
|
|
|
887953 |
+ state->loc_now->parent = inode_ref (parent);
|
|
|
887953 |
+
|
|
|
887953 |
+ if (strchr (resolve->bname, '/')) {
|
|
|
887953 |
+ /* basename should be a string (without '/') in a directory,
|
|
|
887953 |
+ it can't span multiple levels. This can also lead to
|
|
|
887953 |
+ resolving outside the parent's tree, which is not allowed */
|
|
|
887953 |
+ gf_msg (this->name, GF_LOG_ERROR, EPERM,
|
|
|
887953 |
+ PS_MSG_GFID_RESOLVE_FAILED,
|
|
|
887953 |
+ "%s: basename sent by client not allowed",
|
|
|
887953 |
+ resolve->bname);
|
|
|
887953 |
+ resolve->op_ret = -1;
|
|
|
887953 |
+ resolve->op_errno = EPERM;
|
|
|
887953 |
+ ret = 1;
|
|
|
887953 |
+ goto out;
|
|
|
887953 |
+ }
|
|
|
887953 |
state->loc_now->name = resolve->bname;
|
|
|
887953 |
|
|
|
887953 |
inode = inode_grep (state->itable, parent, resolve->bname);
|
|
|
887953 |
diff --git a/xlators/storage/posix/src/posix-handle.h b/xlators/storage/posix/src/posix-handle.h
|
|
|
887953 |
index a0f82ec..45ca1d1 100644
|
|
|
887953 |
--- a/xlators/storage/posix/src/posix-handle.h
|
|
|
887953 |
+++ b/xlators/storage/posix/src/posix-handle.h
|
|
|
887953 |
@@ -223,9 +223,10 @@
|
|
|
887953 |
break; \
|
|
|
887953 |
} \
|
|
|
887953 |
\
|
|
|
887953 |
- if (strstr (loc->name, "../")) { \
|
|
|
887953 |
+ if (strchr (loc->name, '/')) { \
|
|
|
887953 |
gf_msg (this->name, GF_LOG_ERROR, 0, P_MSG_ENTRY_HANDLE_CREATE, \
|
|
|
887953 |
- "'../' in name not allowed: (%s)", loc->name); \
|
|
|
887953 |
+ "'/' in name not allowed: (%s)", loc->name); \
|
|
|
887953 |
+ op_ret = -1; \
|
|
|
887953 |
break; \
|
|
|
887953 |
} \
|
|
|
887953 |
if (LOC_HAS_ABSPATH (loc)) { \
|
|
|
887953 |
--
|
|
|
887953 |
1.8.3.1
|
|
|
887953 |
|