dom96 / httpbeast

A highly performant, multi-threaded HTTP 1.1 server written in Nim.
MIT License
450 stars 53 forks source link

Unsanitized headers can break body in response #56

Open D-Nice opened 3 years ago

D-Nice commented 3 years ago

I was creating some custom headers to add to my response, and was using IDE magic to replicate some lines, which resulted in my last header arguement still containing trailing \c\L chars.

My initial presumption was this would be appropriately handled by the library, but it was not, and caused my resulting response body to be missing characters as the \c\L essentially caused overflow into where the body was expected to start.

An easy solution on my end was

 let sanitizedHeaders = HEADERS.strip(leading = false, chars = {'\c', '\L'})

before inserting the headers in send

I am wondering if you're of the opinion that the library should in fact handle this, or if it's up to the user to sanitize. I understand this is supposed to be performance-oriented and such finer details may be left out for performance reasons, however, it also exposes an unsafeSend and send method, and when such a distinction occurs, I would presume it's pretty hard to shoot myself in the foot with send, yet I accomplished it just via adding some basic headers.

Another option which would lessen the impact on send (instead of doing a check/strip on each call) would be a method exposed to the user to create a HttpBeast safe and compatible header string.

dom96 commented 3 years ago

hm, can you share exactly what the code looked like? i.e. the send call and what the HEADERS contained?

I think the right approach here could be to just add an assert.

D-Nice commented 3 years ago

Yep, the strip was just a naive solution that worked in my case for my specific issue, under /json1 path below, but it doesn't address the underlying issue, which is showcased by /json2:

import options, asyncdispatch, json
import httpbeast

proc onRequest(req: Request): Future[void] =
  if req.httpMethod == some(HttpGet):
    case req.path.get()
    of "/json1":
      const data = $(%*{"message": "Hello, World!"})
      const headers = "Content-Type: application/json\c\L" &
        "Access-Control-Allow-Methods: GET\c\L" &
        "Access-Control-Allow-Headers: Content-Type\c\L" # trailing \c\L cuts off `"}`
      req.send(Http200, data, headers)
    of "/json2":
      const data = $(%*{"message": "Hello, World!"})
      const headers = "Content-Type: application/json\c\L" &
        "Access-Control-Allow-Methods: GET\c\L" &
        "Access-Control-Allow-Headers: Content-Type" &
        "\c\L\c\L" & # causes overflow, can essentially overwrite response data
        "overflows into body"
      req.send(Http200, data, headers)

run(onRequest)

Responses

curl localhost:8080/json1

got


{"message":"Hello, World!

curl localhost:8080/json2

got

overflows into body

{"me

Expected for both

{"message":"Hello, World!"}