From f6ddae40ab7790910b1f454c4a838ba624898900 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 14 May 2018 11:08:59 +0200 Subject: [PATCH] sd-resolve: do not assert on packet size received over a socket This is external data, even if trusted. We should not assert on it, but verify and return proper error instead, which assert_return does. In particular, write(2) says that a partial write could occur when interupted by a signal. When compiled with asserts disabled, we could access memory outside of the allocated buffer. CID #1237671. Follow-up for 1a96c8e1ccb06f87b6bfaff4639390ecd00af588. --- src/libsystemd/sd-resolve/sd-resolve.c | 28 +++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-resolve/sd-resolve.c b/src/libsystemd/sd-resolve/sd-resolve.c index 9fabfcec6a..b5e9c01313 100644 --- a/src/libsystemd/sd-resolve/sd-resolve.c +++ b/src/libsystemd/sd-resolve/sd-resolve.c @@ -321,8 +321,8 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) { req = &packet->rheader; - assert(length >= sizeof(RHeader)); - assert(length == req->length); + assert_return(length >= sizeof(RHeader), -EIO); + assert_return(length == req->length, -EIO); switch (req->type) { @@ -332,8 +332,8 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) { const char *node, *service; int ret; - assert(length >= sizeof(AddrInfoRequest)); - assert(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len); + assert_return(length >= sizeof(AddrInfoRequest), -EBADMSG); + assert_return(length == sizeof(AddrInfoRequest) + ai_req->node_len + ai_req->service_len, -EBADMSG); hints.ai_flags = ai_req->ai_flags; hints.ai_family = ai_req->ai_family; @@ -358,9 +358,9 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) { union sockaddr_union sa; int ret; - assert(length >= sizeof(NameInfoRequest)); - assert(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len); - assert(sizeof(sa) >= ni_req->sockaddr_len); + assert_return(length >= sizeof(NameInfoRequest), -EBADMSG); + assert_return(length == sizeof(NameInfoRequest) + ni_req->sockaddr_len, -EBADMSG); + assert_return(ni_req->sockaddr_len <= sizeof(sa), -EBADMSG); memcpy(&sa, (const uint8_t *) ni_req + sizeof(NameInfoRequest), ni_req->sockaddr_len); @@ -754,11 +754,11 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len int r; assert(resolve); + assert(packet); resp = &packet->rheader; - assert(resp); - assert(length >= sizeof(RHeader)); - assert(length == resp->length); + assert_return(length >= sizeof(RHeader), -EIO); + assert_return(length == resp->length, -EIO); if (resp->type == RESPONSE_DIED) { resolve->dead = true; @@ -780,8 +780,8 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len size_t l; struct addrinfo *prev = NULL; - assert(length >= sizeof(AddrInfoResponse)); - assert(q->type == REQUEST_ADDRINFO); + assert_return(length >= sizeof(AddrInfoResponse), -EBADMSG); + assert_return(q->type == REQUEST_ADDRINFO, -EBADMSG); ASSIGN_ERRNO(q, ai_resp->ret, ai_resp->_errno, ai_resp->_h_errno); @@ -813,8 +813,8 @@ static int handle_response(sd_resolve *resolve, const Packet *packet, size_t len case RESPONSE_NAMEINFO: { const NameInfoResponse *ni_resp = &packet->nameinfo_response; - assert(length >= sizeof(NameInfoResponse)); - assert(q->type == REQUEST_NAMEINFO); + assert_return(length >= sizeof(NameInfoResponse), -EBADMSG); + assert_return(q->type == REQUEST_NAMEINFO, -EBADMSG); if (ni_resp->hostlen > DNS_HOSTNAME_MAX || ni_resp->servlen > DNS_HOSTNAME_MAX || -- 2.25.1