summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAmar Tumballi <amarts@redhat.com>2018-11-01 07:02:11 +0530
committerShyamsundar Ranganathan <srangana@redhat.com>2018-11-05 18:50:10 +0000
commit5441fb4196a94d238377c427f400fe5e28ae5d46 (patch)
tree7ec9af6eb1206663e95d4592f93ed3f92a21404f
parentacb208221bfe3ac897d8eb4fbe18fa6c8aa9286e (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 ba4d2f16a93..26260a5ee2c 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 7c79b569128..c4d7cb14503 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; \
} \