From f0b5816f775ee75d42946694f031e70616a98cd9 Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Thu, 8 Nov 2018 11:02:32 +0530 Subject: server: don't allow '/' in basename Server stack needs to have all the sort of validation, assuming clients can be compromized. It is possible for a compromized client to send basenames with paths with '/', and with that create files without permission on server. By sanitizing the basename, and not allowing anything other than actual directory as the parent for any entry creation, we can mitigate the effects of clients not able to exploit the server. Fixes: CVE-2018-14651 Fixes: bz#1647667 Change-Id: I5dc0da0da2713452ff2b65ac2ddbccf1a267dc20 Signed-off-by: Amar Tumballi --- xlators/protocol/server/src/server-resolve.c | 15 +++++++++++++-- xlators/storage/posix/src/posix-handle.h | 4 ++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/xlators/protocol/server/src/server-resolve.c b/xlators/protocol/server/src/server-resolve.c index b3eda0e4c9e..0fb2ff4e538 100644 --- a/xlators/protocol/server/src/server-resolve.c +++ b/xlators/protocol/server/src/server-resolve.c @@ -308,12 +308,23 @@ resolve_entry_simple (call_frame_t *frame) goto out; } + if (parent->ia_type != IA_IFDIR) { + /* Parent type should be 'directory', and nothing else */ + gf_msg(this->name, GF_LOG_ERROR, EPERM, PS_MSG_GFID_RESOLVE_FAILED, + "%s: parent type not directory (%d)", uuid_utoa(parent->gfid), + parent->ia_type); + resolve->op_ret = -1; + resolve->op_errno = EPERM; + ret = 1; + goto out; + } + /* expected @parent was found from the inode cache */ gf_uuid_copy (state->loc_now->pargfid, resolve->pargfid); state->loc_now->parent = inode_ref (parent); - if (strstr (resolve->bname, "../")) { - /* Resolving outside the parent's tree is not allowed */ + if (strchr (resolve->bname, '/')) { + /* No '/' is allowed in basename. */ gf_msg (this->name, GF_LOG_ERROR, EPERM, PS_MSG_GFID_RESOLVE_FAILED, "%s: path sent by client not allowed", diff --git a/xlators/storage/posix/src/posix-handle.h b/xlators/storage/posix/src/posix-handle.h index 621f8f4d450..04ab0fa1dd5 100644 --- a/xlators/storage/posix/src/posix-handle.h +++ b/xlators/storage/posix/src/posix-handle.h @@ -142,9 +142,9 @@ break; \ } \ \ - if (strstr (loc->name, "../")) { \ + if (strchr (loc->name, '/')) { \ gf_msg (this->name, GF_LOG_ERROR, 0, P_MSG_ENTRY_HANDLE_CREATE, \ - "'../' in name not allowed: (%s)", loc->name); \ + "'/' in name not allowed: (%s)", loc->name); \ op_ret = -1; \ break; \ } \ -- cgit