On Fri, Jan 08, 2016 at 07:29:31PM +0530, P J P wrote:
> +-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+
> | Ah yes, how could I miss that. Maybe just add a min() around the
> | keyname_len computation?
> |
> | - keyname_len = separator ? separator - keys : strlen(keys);
> | + keyname_len = MIN(sizeof(keyname_buf), separator ? separator - keys :
> strlen(keys))
>
> Actually, only use for 'keyname_len' is in the subsequent if statement,
And the replacing of the minus in combined keys.
> which IIUC compares the keyname_buf for "<" key. Maybe it could say
>
> + if (!strncmp(keyname_buf, "<-", 2))
>
> and remove the 'keyname_len' altogether.
This wouldn't catch '<' without '-'. (`sendkey <`)
Also, strncmp with a length of 1 (in the original) seems weird.
In the end my MIN() approach would be too forgiving when a key name is
longer than keyname_buf and its 15-byte substring exists (which I
doubt, though). Ie. {15chars}{and more} would be treated as {15chars}.
Worse with {15chars}{and more}-{anotherkey}.
keyname_len is not useless and perhaps it would be best to just do an
early error check there as I do below.
This makes sure no OOB write happens and changes the error output to
include the 'keys' part instead of keyname_buf. We can do this because
the two other `goto err_out` cases happen after using keyname_buf which
had been pstrcpy()d from the last keys, so 'keys' will contain the same
or more info, and works better for the new case since a string that is
too long might be cut off and then only part of it is displayed if the
input is say a combination of one too-long (thus invalid) key and a
second one.
Alternatively the if() can simply happen after pstrcpy() as a cut-off
error should be good enough anyway.
@@ -1749,6 +1749,9 @@ 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 +1803,7 @@ out:
return;
err_out:
- monitor_printf(mon, "invalid parameter: %s\n", keyname_buf);
+ monitor_printf(mon, "invalid parameter: %s\n", keys);
goto out;
}
