Opened 2 months ago

Closed 2 months ago

#2396 closed defect (invalid)

Disable encoded characters in URIs

Reported by: ymartin-ovh@… Owned by:
Priority: major Milestone:
Component: nginx-core Version: 1.21.x
Keywords: Cc:
uname -a: Linux 5.15.41 x86_64
nginx -V: nginx version: nginx/1.21.6
built by gcc 8.3.0 (Debian 8.3.0-6)
built with OpenSSL 1.1.1d 10 Sep 2019 (running with OpenSSL 1.1.1n 15 Mar 2022)
TLS SNI support enabled
configure arguments: --with-cc-opt='-g -O2 -fdebug-prefix-map=/home/pkg/src/nginx=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -Wno-stringop-overflow -Wno-stringop-truncation' --with-ld-opt='-Wl,-z,relro -fPIC' --conf-path=/etc/nginx/nginx.conf --add-module=../naxsi/naxsi_src/ --with-http_auth_request_module --with-http_geoip_module --with-http_ssl_module --with-http_stub_status_module --with-http_xslt_module --with-http_realip_module --with-stream --with-stream_ssl_module --with-stream_realip_module --with-stream_geoip_module --with-stream_ssl_preread_module --pid-path=/var/run/nginx.pid --lock-path=/var/lock/nginx.lock --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --http-client-body-temp-path=/var/lib/nginx/body --http-proxy-temp-path=/var/lib/nginx/proxy --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --http-scgi-temp-path=/var/lib/nginx/scgi --prefix=/usr --with-debug

Description (last modified by 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

Attachments (1)

encoded-deny.patch (783 bytes ) - added by ymartin-ovh@… 2 months ago.

Download all attachments as: .zip

Change History (4)

by ymartin-ovh@…, 2 months ago

Attachment: encoded-deny.patch added

comment:1 by ymartin-ovh@…, 2 months ago

Description: modified (diff)

comment:2 by ymartin-ovh@…, 2 months ago

Description: modified (diff)

comment:3 by Maxim Dounin, 2 months ago

Resolution: invalid
Status: newclosed

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.

Note: See TracTickets for help on using tickets.