Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

[P J P]

   Hello,

+-- On Fri, 8 Jan 2016, Wolfgang Bumiller wrote --+
| >          if (!strncmp(keyname_buf, "<", 1) && keyname_len == 1) {
| >              pstrcpy(keyname_buf, sizeof(keyname_buf), "less");
| > -            keyname_len = 4;
| 
| keyname_buf is a char[16] so 4 will not overflow it.
| 
| >          }
| > -        keyname_buf[keyname_len] = 0;
| 
| This last write is also used to separate combined keys, so removing
| this write breaks commands such as `sendkeys ctrl-f1`.
| Better add a -1 to the sizeof()s?
| 
| Come to think of it, when is this really an OOB write?
| Given where keyname_len comes from:
| 
| | separator = strchr(keys, '-');
| | keyname_len = separator ? separator - keys : strlen(keys);

  The OOB issue occurs when there is no separator, and strlen(keys) is longer 
than '16' characters. In that case, "keyname_buf[keyname_len] = 0;" writes 
beyond the 'keyname_buf' array. It's removed because 'pstrcpy()' routine also 
null terminates the buffer.

--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F