Il giorno lun 26 gen 2026 alle ore 07:35 Amos Jeffries
<[email protected]> ha scritto:
>
> On 26/01/2026 01:05, Gianluca Cannata wrote:
> > Hi Amos,
> >
> > thank you for your feedback.
> >
> > I have tried to address all of your comments.
> >
> > Let me know what you think.
> >
> > Gianluca
>
> > Index: httpfs/http.c
> > ===================================================================
> > --- httpfs.orig/http.c
> > +++ httpfs/http.c
> > @@ -34,6 +34,8 @@
> > #include <hurd/hurd_types.h>
> > #include <hurd/netfs.h>
> >
> > +#define MAX_HTTP_STATUS_LINE 256
> > +
>
> Still need to document that this value is relatively arbitrary. We only
> use the first 13 bytes of the first line.
What value do you suggest ?
> Technically the HTTP "Reason Phrase" string is unlimited length.
How to protect against it ?
>
> > /* do a DNS lookup for HOST and store result in *DEST */
> > error_t lookup_host (const char *host, struct sockaddr_storage *dest,
> > socklen_t *dest_len, int *socktype, int *protocol)
> > {
> > @@ -100,34 +102,73 @@ error_t lookup_host (const char *host, s
> > return 0;
> > }
> >
> > -/* read the first line from the socket and return an error_t error */
> > +/* read the first line from the socket or return an error_t error */
> > static error_t translate_http_status (int fd, ssize_t *nread)
> > {
> > - char buf[32];
> > + char buf[MAX_HTTP_STATUS_LINE];
> > + size_t i = 0;
> > + ssize_t n;
> > + char c;
> > + int status;
> > +
> > + *nread = 0;
> > +
> > + /* 1. byte-per-byte read until \n */
> > + while (i < sizeof (buf) - 1) {
> > + n = read (fd, &c, 1);
> > + if (n <= 0) return EIO;
> > + buf[i++] = c;
> > + if (c == '\n') break;
> > + }
> >
>
> I suspect this is heading in a bad direction. AFAIK, system calls are
> quite expensive.
>
> We know that a minimum of 13 bytes are needed for this function, so you
> can start with that many and reduce by the amount actually received
> until that much is read in.
Ok.
> After which, you can do whatever to discard the remainder of the line.
> No need to even put that part in buf.
How to discard data ? There is a portable way of doing it ?
> AFAICS the header are currently being dropped so a bulk read to drop the
> status as well is fine ... for now.
>
>
>
> > - *nread = read (fd, buf, sizeof (buf) - 1);
> > - if (*nread < 12) return EIO;
> > - buf[*nread] = '\0';
> > + buf[i] = '\0';
> > + *nread = i;
> >
> > - if (strncmp (buf, "HTTP/", 5) != 0)
> > - return EPROTO;
> > + /* 2. Positional validation: "HTTP/1.N SSS" */
> > + int valid = (i >= 12) &&
> > + (strncmp (buf, "HTTP/1.", 7) == 0) && /* "HTTP/1."
> > */
>
> We know that buf has more than 7 bytes/octets.
> Use memcmp() rather than strncmp().
>
>
> > + (buf[7] >= '0' && buf[7] <= '9') && /* N (digit) */
> > + (buf[8] == ' ') && /* SP */
> > + (buf[9] >= '0' && buf[9] <= '9') && /* S (digit 1)
> > */
> > + (buf[10] >= '0' && buf[10] <= '9') && /* S (digit
> > 2) */
> > + (buf[11] >= '0' && buf[11] <= '9') && /* S (digit
> > 3) */
> > + (buf[12] == ' ' || buf[12] == '\r' || buf[12] ==
> > '\n'); /* End of line */
>
> Sadly bare-CR is a thing from malicious services. We do have to check
> for "\r\n" together instead of just an '\r'.
>
> So IMO the above should be:
>
> (i >= 13) &&
> (memcmp (buf, "HTTP/1.", 7) == 0 && isdigit(buf[7])) && // HTTP version
> (buf[8] == ' ') && // SP delimiter
> (buf[9] >= '1' && buf[9] <= '5' && isdigit(buf[10]) &&
> isdigit(buf[10])) && // status code
> (buf[12] == ' ' || (buf[12] == '\r' && buf[13] == 'n') || buf[12] ==
> '\n'); // SP delimiter or End of Line
>
> Note, with the change to which characters buf[9] is compared against we
> are no longer needing the extra (status < 100 || status >= 600) check below.
>
> >
> > - int status = atoi (buf + 9);
> > + if (!valid) return EPROTO;
> > +
> > + /* 3. HTTP status code conversion */
> > + status = atoi (buf + 9);
> >
> > if (debug_flag)
> > - fprintf (stderr, "HTTP Status: %d\n", status);
> > + fprintf (stderr, "HTTP Protocol Verified. Status: %d\n",
> > status);
> > +
> > + /* 4. RFC 9110 - POSIX mappings */
> > + if (status < 100 || status >= 600) return EPROTO;
> > + if (status < 200) return EIO; /* 1xx not supported */
> > + if (status < 300) return 0; /* 2xx Success */
> > +
> > + if (status < 400) { /* 3xx Redirection */
> > + switch (status) {
> > + case 301: case 302: case 303: case 307: case 308:
> > return EAGAIN;
> > + default: return EIO;
> > + }
> > + }
> > +
> > + if (status < 500) { /* 4xx Client Errors */
> > + switch (status) {
> > + case 401: case 402: case 403: case 405:
> > + case 407: case 421: case 451: return EACCES;
> > + case 404: case 410: case 406: case 412: case 415:
> > case 428: return ENOENT;
> > + case 426: return EPROTO;
> > + default: return EINVAL;
> > + }
> > + }
> >
> > - switch (status)
> > - {
> > - case 200: return 0;
> > - case 301:
> > - case 302: return EAGAIN;
> > - case 401:
> > - case 403: return EACCES;
> > - case 404: return ENOENT;
> > - case 410: return ENOENT;
> > - default:
> > - return (status >= 500) ? EIO : EINVAL;
> > + /* 5xx Server Errors */
> > + switch (status) {
> > + case 501: case 505: case 510: return EINVAL;
> > + case 503: return EACCES;
> > + default: return EIO;
> > }
> > }
>
>
