On Thu, Apr 13, 2023 at 04:12:32PM +0200, Gert Doering wrote:
> Hi,
>
> On Tue, Apr 04, 2023 at 10:32:26AM +0200, Gianmarco De Gregori wrote:
> > diff --git a/src/openvpn/route.c b/src/openvpn/route.c
> > index 3798bc65..00419dce 100644
> > --- a/src/openvpn/route.c
> > +++ b/src/openvpn/route.c
> > @@ -325,7 +325,6 @@ init_route(struct route_ipv4 *r,
> >
> > CLEAR(*r);
> > r->option = ro;
> > -
> > /* network */
> >
> > if (!is_route_parm_defined(ro->network))
> > @@ -437,6 +436,27 @@ init_route(struct route_ipv4 *r,
> >
> > r->flags |= RT_DEFINED;
> >
> > + /* routing table id */
> > +
> > + r->table_id = 0;
> > + if (ro->table_id)
> > + {
> > + r->table_id = atoi(ro->table_id);
> > + if (r->table_id < 0)
> > + {
> > + msg(M_WARN, PACKAGE_NAME "ROUTE: routing table id for network
> > %s (%s) must be >= 0",
>
> Frank's comments alerted me to this, and this certainly is not the way
> to approach it. Syntax checking of the routing table ID must happen during
> option parsing (options.c), not in init_route() - so, this function
> should be able to rely on ro->table_id being an *int*, and properly
> sanitized - "if set, the content is valid".
>
> Same for IPv6, of course.
To be fair, he implemented it in the same way all the other parameters are
implemented. That is why
I did not complain about that (e.g. compare ro->metric, which is treated
exactly the same way).
However, I agree with your general sentiment.
Regards,
--
Frank Lichtenheld
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
