upstream/opnsense-src
1
0
Fork 0
mirror of https://github.com/opnsense/src.git synced 2026-02-19 02:30:08 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

link_addr: be more strict about address formats

Browse source 
instead of accepting any character as a delimiter, only accept ':', '.'
and '-', and only permit a single delimiter in an address.

this prevents accepting bizarre addresses like:

	ifconfig epair2a link 10.1.2.200/28

... which is particularly problematic on an INET6-only system, in which
case ifconfig defaults to the 'link' family, meaning that:

	ifconfig epair2a 10.1.2.200/28

... changes the Ethernet address of the interface.

bump __FreeBSD_version so link_addr() consumers can detect the change.

Reviewed by:	kp, des
Approved by:	des (mentor)
Differential Revision:	https://reviews.freebsd.org/D49936

(cherry picked from commit a1215090416b8afb346fb2ff5b38f25ba0134a3a)

Note-from-OPNsense: not bumping the FreeBSD version for stable/25.7
This commit is contained in:
Lexi Winter 2025-05-14 23:02:59 +01:00 committed by Franco Fichtner
parent 3a5bc7ded9
commit c8759dc5fb
5 changed files with 306 additions and 83 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
lib/libc/net/linkaddr.3
View file

@ -30,7 +30,7 @@
.\"
.\" From: @(#)linkaddr.3 8.1 (Berkeley) 7/28/93
.\"
.Dd May 7, 2025
.Dd May 9, 2025
.Dt LINK_ADDR 3
.Os
.Sh NAME
@ -44,7 +44,7 @@
.In sys/types.h
.In sys/socket.h
.In net/if_dl.h
.Ft void
.Ft int
.Fn link_addr "const char *addr" "struct sockaddr_dl *sdl"
.Ft char *
.Fn link_ntoa "const struct sockaddr_dl *sdl"
@ -53,9 +53,19 @@
.Sh DESCRIPTION
The routine
.Fn link_addr
interprets character strings representing
link-level addresses, returning binary information suitable
for use in system calls.
parses a character string
.Fa addr
representing a link-level address,
and stores the resulting address in the structure pointed to by
.Fa sdl .
A link-level address consists of an optional interface name, followed by
a colon (which is required in all cases), followed by an address
consisting of either a string of hexadecimal digits, or a series of
hexadecimal octets separated by one of the characters
.Sq "." ,
.Sq ":" ,
or
.Sq - .
.Pp
The routine
.Fn link_ntoa
@ -135,10 +145,11 @@ was at least one byte in size.
.Pp
The
.Fn link_addr
function
has no return value.
(See
.Sx BUGS . )
function returns 0 on success.
If the address did not appear to be a valid link-level address, -1 is
returned and
.Va errno
is set to indicate the error.
.Sh SEE ALSO
.Xr getnameinfo 3
.Sh HISTORY
@ -156,11 +167,6 @@ function appeared in
The returned values for link_ntoa
reside in a static memory area.
.Pp
The function
.Fn link_addr
should diagnose improperly formed input, and there should be an unambiguous
way to recognize this.
.Pp
If the
.Va sdl_len
field of the link socket address

198
lib/libc/net/linkaddr.c
View file

@ -39,87 +39,153 @@ static char sccsid[] = "@(#)linkaddr.c 8.1 (Berkeley) 6/4/93";
#include <net/if_dl.h>
#include <assert.h>
#include <errno.h>
#include <stdint.h>
#include <string.h>
/* States*/
#define NAMING 0
#define GOTONE 1
#define GOTTWO 2
#define RESET 3
/* Inputs */
#define DIGIT (4*0)
#define END (4*1)
#define DELIM (4*2)
#define LETTER (4*3)
void
int
link_addr(const char *addr, struct sockaddr_dl *sdl)
{
char *cp = sdl->sdl_data;
char *cplim = sdl->sdl_len + (char *)sdl;
int byte = 0, state = NAMING, new;
const char *nptr;
size_t newsize;
int error = 0;
char delim = 0;
/* Initialise the sdl to zero, except for sdl_len. */
bzero((char *)&sdl->sdl_family, sdl->sdl_len - 1);
sdl->sdl_family = AF_LINK;
do {
state &= ~LETTER;
if ((*addr >= '0') && (*addr <= '9')) {
new = *addr - '0';
} else if ((*addr >= 'a') && (*addr <= 'f')) {
new = *addr - 'a' + 10;
} else if ((*addr >= 'A') && (*addr <= 'F')) {
new = *addr - 'A' + 10;
} else if (*addr == 0) {
state |= END;
} else if (state == NAMING &&
(((*addr >= 'A') && (*addr <= 'Z')) ||
((*addr >= 'a') && (*addr <= 'z'))))
state |= LETTER;
else
state |= DELIM;
addr++;
switch (state /* | INPUT */) {
case NAMING | DIGIT:
case NAMING | LETTER:
*cp++ = addr[-1];
continue;
case NAMING | DELIM:
state = RESET;
sdl->sdl_nlen = cp - sdl->sdl_data;
continue;
case GOTTWO | DIGIT:
*cp++ = byte;
/* FALLTHROUGH */
case RESET | DIGIT:
state = GOTONE;
byte = new;
continue;
case GOTONE | DIGIT:
state = GOTTWO;
byte = new + (byte << 4);
continue;
default: /* | DELIM */
state = RESET;
*cp++ = byte;
byte = 0;
continue;
case GOTONE | END:
case GOTTWO | END:
*cp++ = byte;
/* FALLTHROUGH */
case RESET | END:
/*
* Everything up to the first ':' is the interface name. Usually the
* ':' should always be present even if there's no interface name, but
* since this interface was poorly specified in the past, accept a
* missing colon as meaning no interface name.
*/
if ((nptr = strchr(addr, ':')) != NULL) {
size_t namelen = nptr - addr;
/* Ensure the sdl is large enough to store the name. */
if (namelen > cplim - cp) {
errno = ENOSPC;
return (-1);
}
memcpy(cp, addr, namelen);
cp += namelen;
sdl->sdl_nlen = namelen;
/* Skip the interface name and the colon. */
addr += namelen + 1;
}
/*
* The remainder of the string should be hex digits representing the
* address, with optional delimiters. Each two hex digits form one
* octet, but octet output can be forced using a delimiter, so we accept
* a long string of hex digits, or a mix of delimited and undelimited
* digits like "1122.3344.5566", or delimited one- or two-digit octets
* like "1.22.3".
*
* If anything fails at this point, exit the loop so we set sdl_alen and
* sdl_len based on whatever we did manage to parse. This preserves
* compatibility with the 4.3BSD version of link_addr, which had no way
* to indicate an error and would just return.
*/
#define DIGIT(c) \
(((c) >= '0' && (c) <= '9') ? ((c) - '0') \
: ((c) >= 'a' && (c) <= 'f') ? ((c) - 'a' + 10) \
: ((c) >= 'A' && (c) <= 'F') ? ((c) - 'A' + 10) \
: (-1))
#define ISDELIM(c) (((c) == '.' || (c) == ':' || (c) == '-') && \
(delim == 0 || delim == (c)))
for (;;) {
int digit, digit2;
/*
* Treat any leading delimiters as empty bytes. This supports
* the (somewhat obsolete) form of Ethernet addresses with empty
* octets, e.g. "1::3:4:5:6".
*/
while (ISDELIM(*addr) && cp < cplim) {
delim = *addr++;
*cp++ = 0;
}
/* Did we reach the end of the string? */
if (*addr == '\0')
break;
/*
* If not, the next character must be a digit, so make sure we
* have room for at least one more octet.
*/
if (cp >= cplim) {
error = ENOSPC;
break;
}
break;
} while (cp < cplim);
if ((digit = DIGIT(*addr)) == -1) {
error = EINVAL;
break;
}
++addr;
/* If the next character is another digit, consume it. */
if ((digit2 = DIGIT(*addr)) != -1) {
digit = (digit << 4) | digit2;
++addr;
}
if (ISDELIM(*addr)) {
/*
* If the digit is followed by a delimiter, write it
* and consume the delimiter.
*/
delim = *addr++;
*cp++ = digit;
} else if (DIGIT(*addr) != -1) {
/*
* If two digits are followed by a third digit, treat
* the two digits we have as a single octet and
* continue.
*/
*cp++ = digit;
} else if (*addr == '\0') {
/* If the digit is followed by EOS, we're done. */
*cp++ = digit;
break;
} else {
/* Otherwise, the input was invalid. */
error = EINVAL;
break;
}
}
#undef DIGIT
#undef ISDELIM
/* How many bytes did we write to the address? */
sdl->sdl_alen = cp - LLADDR(sdl);
new = cp - (char *)sdl;
if (new > sizeof(*sdl))
sdl->sdl_len = new;
return;
/*
* The user might have given us an sdl which is larger than sizeof(sdl);
* in that case, record the actual size of the new sdl.
*/
newsize = cp - (char *)sdl;
if (newsize > sizeof(*sdl))
sdl->sdl_len = (u_char)newsize;
if (error == 0)
return (0);
errno = error;
return (-1);
}
char *
link_ntoa(const struct sockaddr_dl *sdl)
{

152
lib/libc/tests/net/link_addr_test.cc
View file

@ -71,10 +71,12 @@ sockaddr_dl
make_linkaddr(const std::string &addr)
{
auto sdl = sockaddr_dl{};
int ret;
sdl.sdl_len = sizeof(sdl);
::link_addr(addr.c_str(), &sdl);
ret = ::link_addr(addr.c_str(), &sdl);
ATF_REQUIRE_EQ(0, ret);
ATF_REQUIRE_EQ(AF_LINK, static_cast<int>(sdl.sdl_family));
ATF_REQUIRE_EQ(true, sdl.sdl_len > 0);
ATF_REQUIRE_EQ(true, sdl.sdl_nlen >= 0);
@ -176,6 +178,10 @@ std::vector<test_address> test_addresses{
{"aa:bb:cc:dd:ee:ff"s, "aa.bb.cc.dd.ee.ff",
ether_addr{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}},
// Address with a blank octet; this is an old form of Ethernet address.
{"00:11::33:44:55"s, "0.11.0.33.44.55",
ether_addr{0x00, 0x11, 0x00, 0x33, 0x44, 0x55}},
};
/*
@ -219,6 +225,43 @@ ATF_TEST_CASE_BODY(ifname)
}
/*
* Test with some invalid addresses.
*/
ATF_TEST_CASE_WITHOUT_HEAD(invalid)
ATF_TEST_CASE_BODY(invalid)
{
auto const invalid_addresses = std::vector{
// Invalid separator
":1/2/3"s,
"ix0:1/2/3"s,
// Multiple different separators
":1.2-3"s,
"ix0:1.2-3"s,
// An IP address
":10.1.2.200/28"s,
"ix0:10.1.2.200/28"s,
// Valid address followed by garbage
":1.2.3xxx"s,
":1.2.3.xxx"s,
"ix0:1.2.3xxx"s,
"ix0:1.2.3.xxx"s,
};
for (auto const &addr : invalid_addresses) {
int ret;
auto sdl = sockaddr_dl{};
sdl.sdl_len = sizeof(sdl);
ret = link_addr(addr.c_str(), &sdl);
ATF_REQUIRE_EQ(-1, ret);
}
}
/*
* Test some non-Ethernet addresses.
*/
@ -245,6 +288,111 @@ ATF_TEST_CASE_BODY(nonether)
lladdr(sdl)));
}
/*
* Test link_addr behaviour with undersized buffers.
*/
ATF_TEST_CASE_WITHOUT_HEAD(smallbuf)
ATF_TEST_CASE_BODY(smallbuf)
{
static constexpr auto garbage = std::byte{0xcc};
auto buf = std::vector<std::byte>();
sockaddr_dl *sdl;
int ret;
/*
* Make an sdl with an sdl_data member of the appropriate size, and
* place it in buf. Ensure it's followed by a trailing byte of garbage
* so we can test that link_addr doesn't write past the end.
*/
auto mksdl = [&buf](std::size_t datalen) -> sockaddr_dl * {
auto actual_size = datalen + offsetof(sockaddr_dl, sdl_data);
buf.resize(actual_size + 1);
std::ranges::fill(buf, garbage);
auto *sdl = new(reinterpret_cast<sockaddr_dl *>(&buf[0]))
sockaddr_dl;
sdl->sdl_len = actual_size;
return (sdl);
};
/* An sdl large enough to store the interface name */
sdl = mksdl(3);
ret = link_addr("ix0:1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(-1, ret);
ATF_REQUIRE_EQ(ENOSPC, errno);
ATF_REQUIRE_EQ(3, sdl->sdl_nlen);
ATF_REQUIRE_EQ("ix0", ifname(*sdl));
ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
/*
* For these tests, test both with and without an interface name, since
* that will overflow the buffer in different places.
*/
/* An empty sdl. Nothing may grow on this cursed ground. */
sdl = mksdl(0);
ret = link_addr("ix0:1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(-1, ret);
ATF_REQUIRE_EQ(ENOSPC, errno);
ATF_REQUIRE_EQ(0, sdl->sdl_nlen);
ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
sdl = mksdl(0);
ret = link_addr(":1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(-1, ret);
ATF_REQUIRE_EQ(ENOSPC, errno);
ATF_REQUIRE_EQ(0, sdl->sdl_nlen);
ATF_REQUIRE_EQ(0, static_cast<int>(sdl->sdl_alen));
/*
* An sdl large enough to store the interface name and two octets of the
* address.
*/
sdl = mksdl(5);
ret = link_addr("ix0:1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(-1, ret);
ATF_REQUIRE_EQ(ENOSPC, errno);
ATF_REQUIRE_EQ("ix0", ifname(*sdl));
ATF_REQUIRE(std::ranges::equal(
std::vector{ 0x01, 0x02 }, lladdr(*sdl)));
sdl = mksdl(2);
ret = link_addr(":1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(-1, ret);
ATF_REQUIRE_EQ(ENOSPC, errno);
ATF_REQUIRE_EQ("", ifname(*sdl));
ATF_REQUIRE(std::ranges::equal(
std::vector{ 0x01, 0x02 }, lladdr(*sdl)));
/*
* An sdl large enough to store the entire address.
*/
sdl = mksdl(6);
ret = link_addr("ix0:1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(0, ret);
ATF_REQUIRE_EQ("ix0", ifname(*sdl));
ATF_REQUIRE(std::ranges::equal(
std::vector{ 0x01, 0x02, 0x03 }, lladdr(*sdl)));
sdl = mksdl(3);
ret = link_addr(":1.2.3", sdl);
ATF_REQUIRE(*rbegin(buf) == garbage);
ATF_REQUIRE_EQ(0, ret);
ATF_REQUIRE_EQ("", ifname(*sdl));
ATF_REQUIRE(std::ranges::equal(
std::vector{ 0x01, 0x02, 0x03 }, lladdr(*sdl)));
}
/*
* Test an extremely long address which would overflow link_ntoa's internal
* buffer. It should handle this by truncating the output.
@ -376,6 +524,8 @@ ATF_INIT_TEST_CASES(tcs)
{
ATF_ADD_TEST_CASE(tcs, basic);
ATF_ADD_TEST_CASE(tcs, ifname);
ATF_ADD_TEST_CASE(tcs, smallbuf);
ATF_ADD_TEST_CASE(tcs, invalid);
ATF_ADD_TEST_CASE(tcs, nonether);
ATF_ADD_TEST_CASE(tcs, overlong);
ATF_ADD_TEST_CASE(tcs, link_ntoa_r);

3
sbin/ifconfig/af_link.c
View file

@ -221,7 +221,8 @@ link_getaddr(const char *addr, int which)
temp[0] = ':';
strcpy(temp + 1, addr);
sdl.sdl_len = sizeof(sdl);
link_addr(temp, &sdl);
if (link_addr(temp, &sdl) == -1)
errx(1, "malformed link-level address");
free(temp);
}
if (sdl.sdl_alen > sizeof(sa->sa_data))

2
sys/net/if_dl.h
View file

@ -86,7 +86,7 @@ struct sockaddr_dl *link_init_sdl(struct ifnet *ifp, struct sockaddr *paddr,
#include <sys/cdefs.h>
__BEGIN_DECLS
void link_addr(const char *, struct sockaddr_dl *);
int link_addr(const char *, struct sockaddr_dl *);
char *link_ntoa(const struct sockaddr_dl *);
int link_ntoa_r(const struct sockaddr_dl *, char *, size_t *);
__END_DECLS