cap_dns, cap_net: fix host and service buffer handling

If a malicious casper process sent a host or service string that was
too long, cap_getnameinfo would overrun the caller's buffer by one byte.

The backends for this function needlessly allocated one extra byte
for these buffers.  This was harmless, but could be confusing to readers.

Reported by:	Coverity (an internal run at Dell)
Reviewed by:	oshogbo, emaste
MFC after:	1 week
Sponsored by:	Dell EMC Isilon
Differential Revision: https://reviews.freebsd.org/D39347
This commit is contained in:
Eric van Gyzen 2023-03-30 17:54:01 -05:00
parent 27f35b7dd4
commit 179bffddf5
3 changed files with 51 additions and 8 deletions

View file

@ -302,9 +302,9 @@ cap_getnameinfo(cap_channel_t *chan, const struct sockaddr *sa, socklen_t salen,
}
if (host != NULL && nvlist_exists_string(nvl, "host"))
strlcpy(host, nvlist_get_string(nvl, "host"), hostlen + 1);
strlcpy(host, nvlist_get_string(nvl, "host"), hostlen);
if (serv != NULL && nvlist_exists_string(nvl, "serv"))
strlcpy(serv, nvlist_get_string(nvl, "serv"), servlen + 1);
strlcpy(serv, nvlist_get_string(nvl, "serv"), servlen);
nvlist_destroy(nvl);
return (0);
}
@ -538,14 +538,14 @@ dns_getnameinfo(const nvlist_t *limits, const nvlist_t *nvlin, nvlist_t *nvlout)
servlen = (size_t)nvlist_get_number(nvlin, "servlen");
if (hostlen > 0) {
host = calloc(1, hostlen + 1);
host = calloc(1, hostlen);
if (host == NULL) {
error = EAI_MEMORY;
goto out;
}
}
if (servlen > 0) {
serv = calloc(1, servlen + 1);
serv = calloc(1, servlen);
if (serv == NULL) {
error = EAI_MEMORY;
goto out;

View file

@ -370,9 +370,9 @@ cap_getnameinfo(cap_channel_t *chan, const struct sockaddr *sa, socklen_t salen,
}
if (host != NULL && nvlist_exists_string(nvl, "host"))
strlcpy(host, nvlist_get_string(nvl, "host"), hostlen + 1);
strlcpy(host, nvlist_get_string(nvl, "host"), hostlen);
if (serv != NULL && nvlist_exists_string(nvl, "serv"))
strlcpy(serv, nvlist_get_string(nvl, "serv"), servlen + 1);
strlcpy(serv, nvlist_get_string(nvl, "serv"), servlen);
nvlist_destroy(nvl);
return (0);
}
@ -879,14 +879,14 @@ net_getnameinfo(const nvlist_t *limits, const nvlist_t *nvlin, nvlist_t *nvlout)
servlen = (size_t)nvlist_get_number(nvlin, "servlen");
if (hostlen > 0) {
host = calloc(1, hostlen + 1);
host = calloc(1, hostlen);
if (host == NULL) {
error = EAI_MEMORY;
goto out;
}
}
if (servlen > 0) {
serv = calloc(1, servlen + 1);
serv = calloc(1, servlen);
if (serv == NULL) {
error = EAI_MEMORY;
goto out;

View file

@ -44,6 +44,8 @@ __FBSDID("$FreeBSD$");
#define TEST_IPV4 "1.1.1.1"
#define TEST_IPV6 "2001:4860:4860::8888"
#define TEST_BIND_IPV4 "127.0.0.1"
#define TEST_PORT 80
#define TEST_PORT_STR "80"
static cap_channel_t *
create_network_service(void)
@ -376,6 +378,45 @@ ATF_TC_BODY(capnet__gethostbyaddr, tc)
cap_close(capnet);
}
ATF_TC_WITHOUT_HEAD(capnet__getnameinfo_buffer);
ATF_TC_BODY(capnet__getnameinfo_buffer, tc)
{
cap_channel_t *chan;
struct sockaddr_in sin;
int ret;
struct {
char host[sizeof(TEST_IPV4)];
char host_canary;
char serv[sizeof(TEST_PORT_STR)];
char serv_canary;
} buffers;
memset(&sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = htons(TEST_PORT);
ret = inet_pton(AF_INET, TEST_IPV4, &sin.sin_addr);
ATF_REQUIRE_EQ(1, ret);
memset(&buffers, '!', sizeof(buffers));
chan = create_network_service();
ret = cap_getnameinfo(chan, (struct sockaddr *)&sin, sizeof(sin),
buffers.host, sizeof(buffers.host),
buffers.serv, sizeof(buffers.serv),
NI_NUMERICHOST | NI_NUMERICSERV);
ATF_REQUIRE_EQ_MSG(0, ret, "%d", ret);
// Verify that cap_getnameinfo worked with minimally sized buffers.
ATF_CHECK_EQ(0, strcmp(TEST_IPV4, buffers.host));
ATF_CHECK_EQ(0, strcmp(TEST_PORT_STR, buffers.serv));
// Verify that cap_getnameinfo did not overflow the buffers.
ATF_CHECK_EQ('!', buffers.host_canary);
ATF_CHECK_EQ('!', buffers.serv_canary);
cap_close(chan);
}
ATF_TC_WITHOUT_HEAD(capnet__limits_addr2name_mode);
ATF_TC_BODY(capnet__limits_addr2name_mode, tc)
{
@ -1248,6 +1289,8 @@ ATF_TP_ADD_TCS(tp)
ATF_TP_ADD_TC(tp, capnet__gethostbyname);
ATF_TP_ADD_TC(tp, capnet__gethostbyaddr);
ATF_TP_ADD_TC(tp, capnet__getnameinfo_buffer);
ATF_TP_ADD_TC(tp, capnet__limits_addr2name_mode);
ATF_TP_ADD_TC(tp, capnet__limits_addr2name_family);
ATF_TP_ADD_TC(tp, capnet__limits_addr2name);