Always treat firmware request and response sizes as unsigned.
This fixes an incomplete bounds check on the guest-supplied request size where a very large request size could be interpreted as a negative value and not be caught by the bounds check. Submitted by: jhb Reported by: Reno Robert Approved by: so Security: FreeBSD-SA-18:14.bhyve Security: CVE-2018-17160
This commit is contained in:
parent
63de13cfee
commit
39040a9ec4
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=341484
@ -79,8 +79,8 @@ static u_int ident_idx;
|
|||||||
|
|
||||||
struct op_info {
|
struct op_info {
|
||||||
int op;
|
int op;
|
||||||
int (*op_start)(int len);
|
int (*op_start)(uint32_t len);
|
||||||
void (*op_data)(uint32_t data, int len);
|
void (*op_data)(uint32_t data, uint32_t len);
|
||||||
int (*op_result)(struct iovec **data);
|
int (*op_result)(struct iovec **data);
|
||||||
void (*op_done)(struct iovec *data);
|
void (*op_done)(struct iovec *data);
|
||||||
};
|
};
|
||||||
@ -119,7 +119,7 @@ errop_set(int err)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static int
|
static int
|
||||||
errop_start(int len)
|
errop_start(uint32_t len)
|
||||||
{
|
{
|
||||||
errop_code = ENOENT;
|
errop_code = ENOENT;
|
||||||
|
|
||||||
@ -128,7 +128,7 @@ errop_start(int len)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
errop_data(uint32_t data, int len)
|
errop_data(uint32_t data, uint32_t len)
|
||||||
{
|
{
|
||||||
|
|
||||||
/* ignore */
|
/* ignore */
|
||||||
@ -188,7 +188,7 @@ static int fget_cnt;
|
|||||||
static size_t fget_size;
|
static size_t fget_size;
|
||||||
|
|
||||||
static int
|
static int
|
||||||
fget_start(int len)
|
fget_start(uint32_t len)
|
||||||
{
|
{
|
||||||
|
|
||||||
if (len > FGET_STRSZ)
|
if (len > FGET_STRSZ)
|
||||||
@ -200,7 +200,7 @@ fget_start(int len)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void
|
static void
|
||||||
fget_data(uint32_t data, int len)
|
fget_data(uint32_t data, uint32_t len)
|
||||||
{
|
{
|
||||||
|
|
||||||
*((uint32_t *) &fget_str[fget_cnt]) = data;
|
*((uint32_t *) &fget_str[fget_cnt]) = data;
|
||||||
@ -285,8 +285,8 @@ static struct req_info {
|
|||||||
struct op_info *req_op;
|
struct op_info *req_op;
|
||||||
int resp_error;
|
int resp_error;
|
||||||
int resp_count;
|
int resp_count;
|
||||||
int resp_size;
|
size_t resp_size;
|
||||||
int resp_off;
|
size_t resp_off;
|
||||||
struct iovec *resp_biov;
|
struct iovec *resp_biov;
|
||||||
} rinfo;
|
} rinfo;
|
||||||
|
|
||||||
@ -346,13 +346,14 @@ fwctl_request_start(void)
|
|||||||
static int
|
static int
|
||||||
fwctl_request_data(uint32_t value)
|
fwctl_request_data(uint32_t value)
|
||||||
{
|
{
|
||||||
int remlen;
|
|
||||||
|
|
||||||
/* Make sure remaining size is >= 0 */
|
/* Make sure remaining size is >= 0 */
|
||||||
|
if (rinfo.req_size <= sizeof(uint32_t))
|
||||||
|
rinfo.req_size = 0;
|
||||||
|
else
|
||||||
rinfo.req_size -= sizeof(uint32_t);
|
rinfo.req_size -= sizeof(uint32_t);
|
||||||
remlen = MAX(rinfo.req_size, 0);
|
|
||||||
|
|
||||||
(*rinfo.req_op->op_data)(value, remlen);
|
(*rinfo.req_op->op_data)(value, rinfo.req_size);
|
||||||
|
|
||||||
if (rinfo.req_size < sizeof(uint32_t)) {
|
if (rinfo.req_size < sizeof(uint32_t)) {
|
||||||
fwctl_request_done();
|
fwctl_request_done();
|
||||||
@ -401,7 +402,7 @@ static int
|
|||||||
fwctl_response(uint32_t *retval)
|
fwctl_response(uint32_t *retval)
|
||||||
{
|
{
|
||||||
uint32_t *dp;
|
uint32_t *dp;
|
||||||
int remlen;
|
ssize_t remlen;
|
||||||
|
|
||||||
switch(rinfo.resp_count) {
|
switch(rinfo.resp_count) {
|
||||||
case 0:
|
case 0:
|
||||||
@ -436,7 +437,7 @@ fwctl_response(uint32_t *retval)
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (rinfo.resp_count > 3 &&
|
if (rinfo.resp_count > 3 &&
|
||||||
rinfo.resp_size - rinfo.resp_off <= 0) {
|
rinfo.resp_off >= rinfo.resp_size) {
|
||||||
fwctl_response_done();
|
fwctl_response_done();
|
||||||
return (1);
|
return (1);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user