On Sat, Jul 15, 2023 at 04:19:27PM +0200, p...@delphinusdns.org wrote:
> >Synopsis: unsafe check for a length in net/rtsock.c
> >Category: kernel
> >Environment:
> System : OpenBSD 7.3
> Details : OpenBSD 7.3 (GENERIC.MP) #2080: Sat Mar 25 14:20:25 MDT
> 2023
>
> dera...@arm64.openbsd.org:/usr/src/sys/arch/arm64/compile/GENERIC.MP
>
> Architecture: OpenBSD.arm64
> Machine : arm64
> >Description:
> While code reading (particularily why RTM_DESYNC says something about
> a buffer overflow, in the route(4) manpage) i bumped into the following in
> net/rtmsock.c:
>
> 500
> 501 /* ensure that we can access the rtm_type via mtod() */
> 502 if (m->m_len < offsetof(struct rt_msghdr, rtm_type) + 1) {
> 503 m_freem(m);
> 504 return;
> 505 }
>
> This may not be sufficient as a safe check because rtm_flags and rtm_tableid
> are
> accessed later in the code, the length check should possibly guarantee the
> entire length of the struct rt_msghdr no?
>
> >How-To-Repeat:
> Code Reading, General Nuisance.
> >Fix:
> I would change the line 502 to:
>
> if (m->m_len < sizeof(struct rt_msghdr)) ...
>
> I don't know if there is any caveats though.
Yes, there are. Not every message sento to route_input() uses struct rt_msghdr.
Also those messages are either generated by the kernel or where checked by
route_output() so the check is not really needed.
I guess the proper fix would be to adjust the rtm_hdrlen check from
route_output() to make sure that a full header is present.
--
:wq Claudio
