From b29b4b4ec846861c975bfa580386d25d48eaa087 Mon Sep 17 00:00:00 2001 From: Ravishankar N Date: Mon, 8 Oct 2018 11:04:14 +0530 Subject: [PATCH 397/399] features/locks: add buffer overflow checks in pl_getxattr Problem: A compromised client can send a variable length buffer value for the GF_XATTR_CLRLK_CMD virtual xattr. If the length is greater than the size of the "key" used to send the response back, locks xlator can segfault when it tries to do a dict_set because of the buffer overflow in strncpy of pl_getxattr(). Fix: Perform size checks while forming the 'key'. Note: This fix is already there in the master branch upstream. Also, it looks like the size PATH_MAX used for 'key' array is not really needed since the maximum length seems to be "strlen(glusterfs.clrlk.tentry.kblocked) + NAME_MAX" where NAME_MAX is used for the basename value in the clear-locks CLI: 'gluster volume clear-locks VOLNAME path kind {blocked | granted | all} {inode range | entry basename | posix range}' But that can be done some other day. Fixes: CVE-2018-14652 BUG: 1634669 Change-Id: I101693e91f9ea2bd26cef6c0b7d82527fefcb3e2 Signed-off-by: Ravishankar N Reviewed-on: https://code.engineering.redhat.com/gerrit/152038 Reviewed-by: Amar Tumballi Reviewed-by: Krutika Dhananjay Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- xlators/features/locks/src/posix.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c index 63bcf31..63f914c 100644 --- a/xlators/features/locks/src/posix.c +++ b/xlators/features/locks/src/posix.c @@ -1120,7 +1120,10 @@ pl_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, goto out; } - strncpy (key, name, strlen (name)); + if (snprintf(key, sizeof(key), "%s", name) >= sizeof(key)) { + op_ret = -1; + goto out; + } if (dict_set_dynstr (dict, key, lk_summary)) { op_ret = -1; op_errno = ENOMEM; -- 1.8.3.1