diff options
| author | Amar Tumballi <amarts@redhat.com> | 2018-11-08 11:02:32 +0530 |
|---|---|---|
| committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2018-11-09 14:04:35 +0000 |
| commit | f0b5816f775ee75d42946694f031e70616a98cd9 (patch) | |
| tree | 223a4fad4eab7c4e3aad751126d072034a311a64 | |
| parent | 9d9b9745c7e424f01e5526b23b1da17db263275e (diff) | |
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 <amarts@redhat.com>
| -rw-r--r-- | xlators/protocol/server/src/server-resolve.c | 15 | ||||
| -rw-r--r-- | 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; \ } \ |