Header parser incorrectly accepts NUL and CR within header values

[kenballus]

From RFC 9110:

Field values containing CR, LF, or NUL characters are invalid and dangerous, due to the varying ways that implementations might parse and interpret those characters; a recipient of CR, LF, or NUL within a field value MUST either reject the message or replace each of those characters with SP before further processing or forwarding of that message.

cpp-httplib does not enforce this rule for CR and NUL. You can see this by running a simple example that echoes back header values (such as this), and sending it a request containing NUL and CR within a header value:

printf 'GET / HTTP/1.1\r\nHost: whatever\r\nTest: \r\x00\r\n\r\n' \
  | timeout 1 ncat --no-shutdown localhost 80 \
  | grep '"headers"' \
  | jq '.["headers"][1][1]' \
  | xargs echo \
  | base64 -d \
  | xxd

00000000: 0d00                                     ..

[yhirose]

@kenballus thanks for the report!

[yhirose]

@kenballus I have merged my pull request for this. Could you test with the latest httplib.h? If you confirm it works, I'll bump the version to v0.17.1, since it's a pretty critical bug. Thanks!

[kenballus]

Confirmed that the new patch works as intended, replacing CR, LF, and NUL with SP.

If you're interested, here's what other implementations do in this situation:

NUL in header value

Reject with 400

• AIOHTTP
• Apache httpd
• FastHTTP
• Go net/http
• Gunicorn
• H2O
• HAProxy
• Hyper
• Hypercorn
• Jetty
• Libmicrohttpd
• Libsoup
• Lighttpd
• Nginx
• Node.js
• LiteSpeed
• Phusion Passenger
• Puma
• Tomcat
• Twisted
• Unicorn
• Uvicorn
• Waitress
• WEBrick
• Microsoft IIS

Close the connection without responding

• Mongoose
• Netty
• protocol-http1
• ServiceTalk
• OpenWrt uhttpd

Accept the header as-is (violates RFC)

• Cheroot
• Dart stdlib
• Tornado

Truncate the header value (violates RFC)

• Libevent
• OpenBSD httpd

Translate to SP

• cpp-httplib

CR in header value

Reject with 400

• AIOHTTP
• Apache httpd
• FastHTTP
• Go net/http
• Gunicorn
• H2O
• HAProxy
• Hyper
• Hypercorn
• Jetty
• Libevent
• Libmicrohttpd
• Lighttpd
• Nginx
• Node.js
• Phusion Passenger
• Puma
• Tomcat
• Unicorn
• Uvicorn
• Waitress
• WEBrick
• Microsoft IIS

Close the connection without responding

• Dart stdlib
• Mongoose
• Netty
• protocol-http1
• ServiceTalk

Accept the header as-is (violates RFC)

• Cheroot
• Tornado
• OpenWrt uhttpd
• OpenBSD httpd

Translate to SP

• cpp-httplib
• Libsoup
• LiteSpeed
• Twisted

LF in header value

Reject with 400

• AIOHTTP
• Apache httpd
• Cheroot
• Gunicorn
• Lighttpd
• Node.js
• Phusion Passenger
• Puma
• Waitress
• WEBrick

Close the connection without responding

• protocol-http1
• ServiceTalk

Accept the header as-is (violates RFC)

• OpenWrt uhttpd
• OpenBSD httpd

Translate to SP

• cpp-httplib
• Twisted

Treat it as a line ending

• Dart stdlib
• FastHTTP
• Go net/http
• H2O
• HAProxy
• Hyper
• Hypercorn
• Jetty
• Libevent
• Libmicrohttpd
• Libsoup
• Mongoose
• Netty
• Nginx
• LiteSpeed
• Tomcat
• Tornado
• Unicorn
• Uvicorn
• Microsoft IIS

[yhirose]

Interesting chart! It looks the majority of implementations reject such requests with 400. I can change the current cpp-httplib behavior to return 400. What do you think?

[kenballus]

If it were me, I'd make it respond 400, but you know your users better than I do. If you think cpp-httplib's use cases warrant extra parsing leniency, then translating to SP might be the right call.

[yhirose]

@yhirose I decided to reject such requests. I'll bump up the version to v0.17.1 soon.
Thanks for your fine contribution!