Disable encoded characters in URIs

[ymartin-ovh@…]

Hello

Similarly to 0b66bd4be777a5b79c5ae0e7dff89fc6429da0fe, maybe the check should be extended to encoded ones.

We have a configuration that use $uri and with the config below, we can trick nginx and change request body:

server {
   listen 127.0.0.1:80;
   server_name _;

   location / {
      proxy_pass http://127.0.0.1:81$uri;
   }
}

server {
   listen 127.0.0.1:81;
   server_name _;
   return 200;
}    

server {
   listen 127.0.0.1:81;
   server_name ko;
   return 418;
}

curl -v 'http://localhost/toto' => < HTTP/1.1 200 OK

curl -v 'http://localhost/toto%20HTTP/1.1%0d%0aHost:ko%0d%0a%0d%0a'
=>
> GET /toto%20HTTP/1.1%0d%0aHost:ko%0d%0a%0d%0a HTTP/1.1
> Host: localhost
> User-Agent: curl/7.64.0
> Accept: */*
> 
< HTTP/1.1 418 
< Server: nginx/1.21.6
< Date: Sat, 01 Oct 2022 12:55:15 GMT
< Content-Length: 0
< Connection: keep-alive
< 

The attached patch tries to mimic fix on the commit 0b66bd4be777a5b79c5ae0e7dff89fc6429da0fe.

Same crafted curl gives me now:
< HTTP/1.1 400 Bad Request
< Server: nginx/1.21.6

What do you think ?

Regards

[Maxim Dounin]

In no particular order:

• The configuration in your snippet is vulnerable to HTTP request splitting, since $uri is unescaped, while proxy_pass expects properly escaped argument. And this is what your example request demonstrates.

In this particular configuration proxy_pass http://127.0.0.1:81; would be enough and won't have the vulnerability you've introduced by using $uri.

• Escaped control characters are quite normal and are expected to appear in requests in many valid cases. There are no reasons to reject such requests.

• Trac is a wrong place to suggest patches, please use nginx-devel@ mailing list instead. See ​here for more hints.

Hope this helps.