Page MenuHomeFreeBSD

Fix buffer overrun in getrpcport(3)
ClosedPublic

Authored by trasz on Nov 23 2020, 1:28 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 3:44 PM
Unknown Object (File)
Mon, Oct 21, 5:41 PM
Unknown Object (File)
Sun, Oct 20, 3:52 PM
Unknown Object (File)
Sun, Oct 20, 5:23 AM
Unknown Object (File)
Sun, Oct 20, 5:23 AM
Unknown Object (File)
Sun, Oct 20, 5:22 AM
Unknown Object (File)
Sun, Oct 20, 5:22 AM
Unknown Object (File)
Sun, Oct 20, 4:59 AM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

trasz requested review of this revision.Nov 23 2020, 1:28 PM
lib/libc/rpc/getrpcport.c
72

I don't really see why this is sufficient validation. We are not checking h_addrtype, so if gethostbyname() isn't returning an IPv4 address, we're just going to copy a bogus value here. If the intent is to guard against a bug in gethostbyname() we should also check for a negative length.

lib/libc/rpc/getrpcport.c
72

My assumption, based on the gethostbyname(3) man page, was that it only ever returns AF_INET addresses, as opposed to gesthostbyname(2). Looking at the source code, though, it appears not necessarily being the case. I wonder what should we do here - use gesthostbyname2 with AF_INET, or also handle ipv6 addresses? Can getrpcport() callers handle inet6?

lib/libc/rpc/getrpcport.c
72

Here we're just returning a port number, so I don't see why callers would care which address type is used. The libc code which invokes the GETPORT RPC on the caller's behalf is certainly IPv4 only, but rpcbind knows about IPv6, so it could be fixed without a lot of difficult I think.

For the purpose of this code review, I think using gethostbyname2() as you suggest is fine. The only users of getrpcport() in the base system are the NIS programs, which are IPv4-only I believe.

Make sure to request IPv4 address.

This revision is now accepted and ready to land.Dec 11 2020, 3:10 PM

(Tinderboxed, except for MIPS.)

This revision was automatically updated to reflect the committed changes.