summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmar Tumballi <amarts@redhat.com>2018-11-01 07:02:11 +0530
committerAmar Tumballi <amarts@redhat.com>2018-11-01 15:04:45 +0530
commit5fdb7ae37f602894f81a2cadc5a4c609a4c85427 (patch)
treed9f7db1c6f6b66772475f3f796fdf02b2b766a6d
parent7fac81aeab5805fb2bd719d7489636633bb5e32a (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#1644755 Change-Id: I5dc0da0da2713452ff2b65ac2ddbccf1a267dc20 Signed-off-by: Amar Tumballi <amarts@redhat.com>
-rw-r--r--xlators/protocol/server/src/server-resolve.c21
-rw-r--r--xlators/storage/posix/src/posix-handle.h4
2 files changed, 19 insertions, 6 deletions
diff --git a/xlators/protocol/server/src/server-resolve.c b/xlators/protocol/server/src/server-resolve.c
index ba4d2f1..26260a5 100644
--- a/xlators/protocol/server/src/server-resolve.c
+++ b/xlators/protocol/server/src/server-resolve.c
@@ -294,19 +294,32 @@ 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, '/')) {
+ /* basename should be a string (without '/') in a directory,
+ it can't span multiple levels. This can also lead to
+ resolving outside the parent's tree, which is not allowed */
gf_msg(this->name, GF_LOG_ERROR, EPERM, PS_MSG_GFID_RESOLVE_FAILED,
- "%s: path sent by client not allowed", resolve->bname);
+ "%s: basename sent by client not allowed", resolve->bname);
resolve->op_ret = -1;
resolve->op_errno = EPERM;
ret = 1;
goto out;
}
+
state->loc_now->name = resolve->bname;
inode = inode_grep(state->itable, parent, resolve->bname);
diff --git a/xlators/storage/posix/src/posix-handle.h b/xlators/storage/posix/src/posix-handle.h
index 7c79b56..c4d7cb1 100644
--- a/xlators/storage/posix/src/posix-handle.h
+++ b/xlators/storage/posix/src/posix-handle.h
@@ -150,9 +150,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; \
} \