On Sat Jul 18, 2026 at 07:27:07PM +0200, Kirill A. Korinsky wrote:
> On Sat, 18 Jul 2026 15:25:00 +0200,
> Rafael Sadowski <[email protected]> wrote:
> >
> > On Sun Jul 12, 2026 at 03:15:15PM +0200, Kirill A. Korinsky wrote:
> > > On Sat, 04 Jul 2026 00:27:00 +0200,
> > > Andrew Kloet <[email protected]> wrote:
> > > >
> > > > On June 18, 2026 7:44:31 PM UTC, Andrew Kloet <[email protected]> wrote:
> > > > >>Synopsis: HTTP suffix-range requests omits file contents
> > > > >>Category: user
> > > > >>Environment:
> > > > > System : OpenBSD 7.9
> > > > > Details : OpenBSD 7.9 (GENERIC.MP) #449: Wed May 6
> > > > > 13:17:25 MDT 2026
> > > > >
> > > > > [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > > > >
> > > > > Architecture: OpenBSD.amd64
> > > > > Machine : amd64
> > > > >>Description:
> > > > > There is a logic flaw inside httpd's parse_range_sepc when
> > > > > processing HTTP Byte-Range Requests.
> > > > >
> > > > > When a client issues a suffix-range request (Range: bytes=-num
> > > > > to
> > > > > request the trailing num bytes of a resource) where the
> > > > > requested
> > > > > count is larger than the target file size, an overflow guard
> > > > > caps
> > > > > r->end to size - 1 prematurely.
> > > > >
> > > > > Because this truncation happens before the evaluation block
> > > > > calculates the starting byte offset (r->start = size - r->end),
> > > > > the
> > > > > resulting offset math evaluates to 1 instead of 0. As a result,
> > > > > httpd serves a 206 Partial Content response that completely
> > > > > omits
> > > > > the very first byte (offset 0) of the target file.
> > > > >>How-To-Repeat:
> > > > > # Create an 11-byte file
> > > > > $ echo "0123456789" > /var/www/htdocs/test
> > > > >
> > > > > # Send an HTTP request with a suffix range exceeding the file
> > > > > size.
> > > > > # Note the response headers show an incorrect range and omit the
> > > > > # leading character
> > > > > $ curl -v -H "Range: bytes=-15" http://127.0.0.1/test
> > > > > < HTTP/1.1 206 Partial Content
> > > > > < Content-Range: bytes 1-10/11
> > > > > < Content-Length: 10
> > > > > 123456789
> > > > >>Fix:
> > > > > Decouple the suffix range processing from standard ranges within
> > > > > parse_range_spec so that ceiling adjustments to r->end don't
> > > > > corrupt
> > > > > the initial offset computation.
> > > > >
> > > > >--- a/usr.sbin/httpd/server_file.c
> > > > >+++ b/usr.sbin/httpd/server_file.c
> > > > >@@ -797,15 +797,18 @@ parse_range_spec(char *str, size_t size, struct
> > > > >range *r)
> > > > > if ((start_str_len == 0) && (end_str_len == 0))
> > > > > return (0);
> > > > >
> > > > >- if (end_str_len) {
> > > > >+ if (start_str_len == 0) {
> > > > > r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> > > > >- if (errstr)
> > > > >+ if (errstr || r->end == 0)
> > > > > return (0);
> > > > >
> > > > >- if ((size_t)r->end >= size)
> > > > >- r->end = size - 1;
> > > > >- } else
> > > > >+ if ((size_t)r->end > size)
> > > > >+ r->start = 0;
> > > > >+ else
> > > > >+ r->start = size - r->end;
> > > > > r->end = size - 1;
> > > > >+ return (1);
> > > > >+ }
> > > > >
> > > > > if (start_str_len) {
> > > > > r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> > > > >@@ -814,11 +817,17 @@ parse_range_spec(char *str, size_t size, struct
> > > > >range *r)
> > > > >
> > > > > if ((size_t)r->start >= size)
> > > > > return (0);
> > > > >- } else {
> > > > >- r->start = size - r->end;
> > > > >- r->end = size - 1;
> > > > > }
> > > > >
> > > > >+ if (end_str_len) {
> > > > >+ r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> > > > >+ if (errstr)
> > > > >+ return (0);
> > > > >+ if ((size_t)r->end >= size)
> > > > >+ r->end = size - 1;
> > > > >+ } else
> > > > >+ r->end = size - 1;
> > > > >+
> > > > > if (r->end < r->start)
> > > > > return (0);
> > > >
> > > > bump
> > > >
> > >
> > > I think you have fixed only one edge case. After spending some time on
> > > this,
> > > I had this diff which fixed a few more cases plus addresses tests.
> > >
> > > Here, I calculate suffix ranges before clamping end offsets so requests at
> > > least as large as the file return the complete representation. Reject zero
> > > length and malformed ranges, retain 64 bit sizes throughout range
> > > handling,
> > > and add regression coverage for boundaries and overflow.
> > >
> > > Ok?
> >
> > A thought on this. You could keep the strtonum and compare the errstr
> > strcmp(errstr, "too large"). I think that would make the code easier to
> > read and you safe the errno dance, but that's just my personal opinion.
> >
> > It’s up to you to decide, OK rsadowski if you want go with this version.
> >
>
> You're right, that make that error dance simpler.
>
> Ok?
OK rsadowski
>
> Index: usr.sbin/httpd/httpd.h
> ===================================================================
> RCS file: /home/cvs/src/usr.sbin/httpd/httpd.h,v
> diff -u -p -r1.178 httpd.h
> --- usr.sbin/httpd/httpd.h 2 Jul 2026 04:59:16 -0000 1.178
> +++ usr.sbin/httpd/httpd.h 12 Jul 2026 12:19:37 -0000
> @@ -280,7 +280,7 @@ struct range_data {
>
> /* For the Content headers in each part */
> struct media_type *range_media;
> - size_t range_total;
> + off_t range_total;
> };
>
> struct client {
> Index: usr.sbin/httpd/server_file.c
> ===================================================================
> RCS file: /home/cvs/src/usr.sbin/httpd/server_file.c,v
> diff -u -p -r1.82 server_file.c
> --- usr.sbin/httpd/server_file.c 21 Jun 2026 19:23:56 -0000 1.82
> +++ usr.sbin/httpd/server_file.c 18 Jul 2026 17:17:51 -0000
> @@ -52,8 +52,8 @@ int server_file_index(struct httpd *,
> int server_file_modified_since(struct http_descriptor *,
> const struct timespec *);
> int server_file_method(struct client *);
> -int parse_range_spec(char *, size_t, struct range *);
> -int parse_ranges(struct client *, char *, size_t);
> +int parse_range_spec(char *, off_t, struct range *);
> +int parse_ranges(struct client *, char *, off_t);
> static int select_visible(const struct dirent *);
>
> int
> @@ -351,7 +351,8 @@ server_partial_file_request(struct httpd
> struct media_type multipart_media;
> struct range_data *r = &clt->clt_ranges;
> struct range *range;
> - size_t content_length = 0, bufsiz;
> + off_t content_length = 0, range_length;
> + size_t bufsiz;
> int code = 500, i, nranges, ret;
> char content_range[64];
> const char *errstr = NULL;
> @@ -396,12 +397,22 @@ server_partial_file_request(struct httpd
> goto abort;
>
> /* Add data length */
> - content_length += ret + range->end - range->start + 1;
> + range_length = range->end - range->start + 1;
> + if (content_length > LLONG_MAX - ret ||
> + range_length > LLONG_MAX - content_length - ret) {
> + errno = EOVERFLOW;
> + goto abort;
> + }
> + content_length += ret + range_length;
>
> }
> if ((ret = snprintf(NULL, 0, "\r\n--%llu--\r\n",
> clt->clt_boundary)) < 0)
> goto abort;
> + if (content_length > LLONG_MAX - ret) {
> + errno = EOVERFLOW;
> + goto abort;
> + }
> content_length += ret;
>
> /* prepare multipart/byteranges media type */
> @@ -743,7 +754,7 @@ server_file_modified_since(struct http_d
> }
>
> int
> -parse_ranges(struct client *clt, char *str, size_t file_sz)
> +parse_ranges(struct client *clt, char *str, off_t file_sz)
> {
> int i = 0;
> char *p, *q;
> @@ -783,9 +794,10 @@ parse_ranges(struct client *clt, char *s
> }
>
> int
> -parse_range_spec(char *str, size_t size, struct range *r)
> +parse_range_spec(char *str, off_t size, struct range *r)
> {
> size_t start_str_len, end_str_len;
> + off_t value;
> char *p, *start_str, *end_str;
> const char *errstr;
>
> @@ -802,27 +814,41 @@ parse_range_spec(char *str, size_t size,
> if ((start_str_len == 0) && (end_str_len == 0))
> return (0);
>
> - if (end_str_len) {
> - r->end = strtonum(end_str, 0, LLONG_MAX, &errstr);
> - if (errstr)
> - return (0);
> + if (size == 0)
> + return (0);
>
> - if ((size_t)r->end >= size)
> - r->end = size - 1;
> - } else
> + if (start_str_len == 0) {
> + if (*end_str < '0' || *end_str > '9')
> + return (0);
> + value = strtonum(end_str, 0, size, &errstr);
> + if (errstr != NULL && errno != ERANGE)
> + return (0);
> + if (errstr != NULL)
> + r->start = 0;
> + else if (value == 0)
> + return (0);
> + else
> + r->start = size - value;
> r->end = size - 1;
> + return (1);
> + }
>
> - if (start_str_len) {
> - r->start = strtonum(start_str, 0, LLONG_MAX, &errstr);
> - if (errstr)
> - return (0);
> + if (*start_str < '0' || *start_str > '9')
> + return (0);
> + r->start = strtonum(start_str, 0, size - 1, &errstr);
> + if (errstr != NULL)
> + return (0);
>
> - if ((size_t)r->start >= size)
> + if (end_str_len) {
> + if (*end_str < '0' || *end_str > '9')
> return (0);
> - } else {
> - r->start = size - r->end;
> + r->end = strtonum(end_str, 0, size - 1, &errstr);
> + if (errstr != NULL && errno != ERANGE)
> + return (0);
> + if (errstr != NULL)
> + r->end = size - 1;
> + } else
> r->end = size - 1;
> - }
>
> if (r->end < r->start)
> return (0);
> Index: usr.sbin/httpd/server_http.c
> ===================================================================
> RCS file: /home/cvs/src/usr.sbin/httpd/server_http.c,v
> diff -u -p -r1.167 server_http.c
> --- usr.sbin/httpd/server_http.c 2 Jul 2026 04:59:50 -0000 1.167
> +++ usr.sbin/httpd/server_http.c 12 Jul 2026 13:04:29 -0000
> @@ -724,7 +724,7 @@ server_read_httprange(struct bufferevent
> if (server_bufferevent_printf(clt,
> "\r\n--%llu\r\n"
> "Content-Type: %s/%s\r\n"
> - "Content-Range: bytes %lld-%lld/%zu\r\n\r\n",
> + "Content-Range: bytes %lld-%lld/%lld\r\n\r\n",
> clt->clt_boundary,
> media->media_type, media->media_subtype,
> range->start, range->end, r->range_total) == -1)
>
>
> --
> wbr, Kirill
>
