Fix buffer overflow when processing crafted command packets

When the length of the REQ_SUBNETS_ACCESSED, REQ_CLIENT_ACCESSES
command requests and the RPY_SUBNETS_ACCESSED, RPY_CLIENT_ACCESSES,
RPY_CLIENT_ACCESSES_BY_INDEX, RPY_MANUAL_LIST command replies is
calculated, the number of items stored in the packet is not validated.

A crafted command request/reply can be used to crash the server/client.
Only clients allowed by cmdallow (by default only localhost) can crash
the server.

With chrony versions 1.25 and 1.26 this bug has a smaller security
impact as the server requires the clients to be authenticated in order
to process the subnet and client accesses commands. In 1.27 and 1.28,
however, the invalid calculated length is included also in the
authentication check which may cause another crash.
This commit is contained in:
Miroslav Lichvar 2013-07-31 15:01:15 +02:00
parent a9a5f98406
commit 7712455d9a
3 changed files with 41 additions and 21 deletions

View file

@ -1371,7 +1371,8 @@ submit_request(CMD_Request *request, CMD_Reply *reply, int *reply_auth_ok)
read_length = recvfrom_status;
expected_length = PKL_ReplyLength(reply);
bad_length = (read_length < expected_length);
bad_length = (read_length < expected_length ||
expected_length < offsetof(CMD_Reply, data));
bad_sender = (where_from.u.sa_family != his_addr.u.sa_family ||
(where_from.u.sa_family == AF_INET &&
(where_from.in4.sin_addr.s_addr != his_addr.in4.sin_addr.s_addr ||

View file

@ -1781,29 +1781,10 @@ read_from_cmd_socket(void *anything)
}
read_length = status;
expected_length = PKL_CommandLength(&rx_message);
rx_command = ntohs(rx_message.command);
LCL_ReadRawTime(&now);
LCL_CookTime(&now, &cooked_now, NULL);
tx_message.version = PROTO_VERSION_NUMBER;
tx_message.pkt_type = PKT_TYPE_CMD_REPLY;
tx_message.res1 = 0;
tx_message.res2 = 0;
tx_message.command = rx_message.command;
tx_message.sequence = rx_message.sequence;
tx_message.reply = htons(RPY_NULL);
tx_message.number = htons(1);
tx_message.total = htons(1);
tx_message.pad1 = 0;
tx_message.utoken = htonl(utoken);
/* Set this to a default (invalid) value. This protects against the
token field being set to an arbitrary value if we reject the
message, e.g. due to the host failing the access check. */
tx_message.token = htonl(0xffffffffUL);
memset(&tx_message.auth, 0, sizeof(tx_message.auth));
switch (where_from.u.sa_family) {
case AF_INET:
remote_ip.family = IPADDR_INET4;
@ -1830,7 +1811,14 @@ read_from_cmd_socket(void *anything)
allowed = ADF_IsAllowed(access_auth_table, &remote_ip) || localhost;
if (read_length < offsetof(CMD_Request, data) ||
/* Message size sanity check */
if (read_length >= offsetof(CMD_Request, data)) {
expected_length = PKL_CommandLength(&rx_message);
} else {
expected_length = 0;
}
if (expected_length < offsetof(CMD_Request, data) ||
rx_message.pkt_type != PKT_TYPE_CMD_REQUEST ||
rx_message.res1 != 0 ||
rx_message.res2 != 0) {
@ -1842,6 +1830,25 @@ read_from_cmd_socket(void *anything)
return;
}
rx_command = ntohs(rx_message.command);
tx_message.version = PROTO_VERSION_NUMBER;
tx_message.pkt_type = PKT_TYPE_CMD_REPLY;
tx_message.res1 = 0;
tx_message.res2 = 0;
tx_message.command = rx_message.command;
tx_message.sequence = rx_message.sequence;
tx_message.reply = htons(RPY_NULL);
tx_message.number = htons(1);
tx_message.total = htons(1);
tx_message.pad1 = 0;
tx_message.utoken = htonl(utoken);
/* Set this to a default (invalid) value. This protects against the
token field being set to an arbitrary value if we reject the
message, e.g. due to the host failing the access check. */
tx_message.token = htonl(0xffffffffUL);
memset(&tx_message.auth, 0, sizeof(tx_message.auth));
if (rx_message.version != PROTO_VERSION_NUMBER) {
tx_message.status = htons(STT_NOHOSTACCESS);
if (!LOG_RateLimited()) {

View file

@ -127,6 +127,8 @@ PKL_CommandLength(CMD_Request *r)
{
unsigned long ns;
ns = ntohl(r->data.subnets_accessed.n_subnets);
if (ns > MAX_SUBNETS_ACCESSED)
return 0;
return (offsetof(CMD_Request, data.subnets_accessed.subnets) +
ns * sizeof(REQ_SubnetsAccessed_Subnet));
}
@ -134,6 +136,8 @@ PKL_CommandLength(CMD_Request *r)
{
unsigned long nc;
nc = ntohl(r->data.client_accesses.n_clients);
if (nc > MAX_CLIENT_ACCESSES)
return 0;
return (offsetof(CMD_Request, data.client_accesses.client_ips) +
nc * sizeof(unsigned long));
}
@ -197,6 +201,8 @@ PKL_ReplyLength(CMD_Reply *r)
{
unsigned long ns = ntohl(r->data.subnets_accessed.n_subnets);
if (r->status == htons(STT_SUCCESS)) {
if (ns > MAX_SUBNETS_ACCESSED)
return 0;
return (offsetof(CMD_Reply, data.subnets_accessed.subnets) +
ns * sizeof(RPY_SubnetsAccessed_Subnet));
} else {
@ -207,6 +213,8 @@ PKL_ReplyLength(CMD_Reply *r)
{
unsigned long nc = ntohl(r->data.client_accesses.n_clients);
if (r->status == htons(STT_SUCCESS)) {
if (nc > MAX_CLIENT_ACCESSES)
return 0;
return (offsetof(CMD_Reply, data.client_accesses.clients) +
nc * sizeof(RPY_ClientAccesses_Client));
} else {
@ -217,6 +225,8 @@ PKL_ReplyLength(CMD_Reply *r)
{
unsigned long nc = ntohl(r->data.client_accesses_by_index.n_clients);
if (r->status == htons(STT_SUCCESS)) {
if (nc > MAX_CLIENT_ACCESSES)
return 0;
return (offsetof(CMD_Reply, data.client_accesses_by_index.clients) +
nc * sizeof(RPY_ClientAccesses_Client));
} else {
@ -226,6 +236,8 @@ PKL_ReplyLength(CMD_Reply *r)
case RPY_MANUAL_LIST:
{
unsigned long ns = ntohl(r->data.manual_list.n_samples);
if (ns > MAX_MANUAL_LIST_SAMPLES)
return 0;
if (r->status == htons(STT_SUCCESS)) {
return (offsetof(CMD_Reply, data.manual_list.samples) +
ns * sizeof(RPY_ManualListSample));