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?
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
