On Sun, Jan 10, 2016 at 10:56:55AM +0300, Michael Tokarev wrote:
> So, what's the status of this issue now?
> (it is CVE-2015-8619 btw, maybe worth to mention this in the commit message)
Seems we concluded it's best to keep keyname_len around and simply check
it against the sizeof(keyname_buf).
Here's a full new version as I haven't seen one yet. (With an adapted
commit message and the CVE id added.)
I did not include the proposed change to the pstrcpy() size parameter
as it seemed more like a coding-style change and because the code also
uses
pstrcpy(keyname_buf, sizeof(keyname_buf), "less")
instead of a memcpy() (after all, the buffer size is known and the
contents are constant in that line).
Patch:
===
>From 8da4a3bf8fb076314f986a0d58cb94f5458e3659 Mon Sep 17 00:00:00 2001
From: Wolfgang Bumiller <w.bumil...@proxmox.com>
Date: Mon, 11 Jan 2016 08:21:25 +0100
Subject: [PATCH] hmp: fix sendkey out of bounds write (CVE-2015-8619)
When processing 'sendkey' command, hmp_sendkey routine null
terminates the 'keyname_buf' array. This results in an OOB
write issue, if 'keyname_len' was to fall outside of
'keyname_buf' array.
Now checking the length against the buffer size before using
it.
Reported-by: Ling Liu <liuling...@360.cn>
Signed-off-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
---
hmp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index c2b2c16..0c7a04c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1749,6 +1749,8 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
while (1) {
separator = strchr(keys, '-');
keyname_len = separator ? separator - keys : strlen(keys);
+ if (keyname_len >= sizeof(keyname_buf))
+ goto err_out;
pstrcpy(keyname_buf, sizeof(keyname_buf), keys);
/* Be compatible with old interface, convert user inputted "<" */
@@ -1800,7 +1802,7 @@ out:
return;
err_out:
- monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
+ monitor_printf(mon, "invalid parameter: %s\n", keys);
goto out;
}
--
2.1.4
