On Wed, Nov 02, 2022 at 11:45:43AM +0100, Theo Buehler wrote: > Not all callers of valid_uri() ensure that the uri passed in is actually > a C string and the API implies at least that uri[usz - 1] != '\0' is > allowed. For example, x509_location() a priori doesn't pass a C string > and Job will soon add a second instance. I think we should explicitly > length check uri to be long enough before calling strncasecmp() on it. > > The diff below is mostly cosmetic as it happens to be the case that > libcrypto NUL-terminates ASN.1 strings for historical reasons, but this > is an implementation detail and design mistake (general ASN.1 strings > can have embedded NULs) that we shouldn't rely on. > > The diff does change behavior in that a naked "https://"; will no longer > be considered a valid uri (which I'm pretty sure it isn't, but I may > have missed a corner case in the spec). > > Index: validate.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v > retrieving revision 1.45 > diff -u -p -r1.45 validate.c > --- validate.c 3 Sep 2022 14:41:47 -0000 1.45 > +++ validate.c 2 Nov 2022 10:20:40 -0000 > @@ -290,6 +290,8 @@ valid_uri(const char *uri, size_t usz, c > > if (proto != NULL) { > s = strlen(proto); > + if (s >= usz) > + return 0; > if (strncasecmp(uri, proto, s) != 0) > return 0; > } >
Makes sense. OK claudio@ Also "https://"; should be considered invalid since there is no host specified. -- :wq Claudio
