Opened 22 months ago

Closed 21 months ago

Last modified 21 months ago

#2452 closed enhancement (wontfix)

fastcgi_split_path_info should specify NGX_REGEX_MULTILINE

Reported by: kohler@… Owned by:
Priority: minor Milestone:
Component: nginx-module Version: 1.19.x
Keywords: Cc: kohler@…
uname -a: Darwin Eddies-MacBook-Pro-4.local 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:04:44 PST 2022; root:xnu-8792.61.2~4/RELEASE_ARM64_T8103 arm64
nginx -V: nginx version: nginx/1.23.3
built by clang 14.0.0 (clang-1400.0.29.202)
built with OpenSSL 1.1.1s 1 Nov 2022
TLS SNI support enabled
configure arguments: --prefix=/opt/homebrew/Cellar/nginx/1.23.3 --sbin-path=/opt/homebrew/Cellar/nginx/1.23.3/bin/nginx --with-cc-opt='-I/opt/homebrew/opt/pcre2/include -I/opt/homebrew/opt/openssl@1.1/include' --with-ld-opt='-L/opt/homebrew/opt/pcre2/lib -L/opt/homebrew/opt/openssl@1.1/lib' --conf-path=/opt/homebrew/etc/nginx/nginx.conf --pid-path=/opt/homebrew/var/run/nginx.pid --lock-path=/opt/homebrew/var/run/nginx.lock --http-client-body-temp-path=/opt/homebrew/var/run/nginx/client_body_temp --http-proxy-temp-path=/opt/homebrew/var/run/nginx/proxy_temp --http-fastcgi-temp-path=/opt/homebrew/var/run/nginx/fastcgi_temp --http-uwsgi-temp-path=/opt/homebrew/var/run/nginx/uwsgi_temp --http-scgi-temp-path=/opt/homebrew/var/run/nginx/scgi_temp --http-log-path=/opt/homebrew/var/log/nginx/access.log --error-log-path=/opt/homebrew/var/log/nginx/error.log --with-compat --with-debug --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_degradation_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-ipv6 --with-mail --with-mail_ssl_module --with-pcre --with-pcre-jit --with-stream --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module

Description

An HTTP request such as “GET /blah.php/%0D%0A.gif” may contain encoded newlines. Since fastcgi_split_path_info does not set the NGX_REGEX_MULTILINE flag on split_regex, these requests will interact badly with widely-recommended mod_fastcgi configurations. For example, nginx documentation suggests a configuration like this:

location ~ [^/]\.php(/|$) {
    fastcgi_split_path_info ^(.+?\.php)(/.*)$;

A GET /blah.php/%0D%0A.gif request will not match the fastcgi_split_path_info regex, because the . meta-character doesn't match the encoded newline. As a result, a bogus value is computed for SCRIPT_FILENAME. Depending on the rest of the configuration, nginx will return an error response or pass an incorrect script value to the fastcgi server.

This can be worked around, such as with

    fastcgi_split_path_info ^(.+?\.php)(/[\s\S]*)$;

but I feel it would be much less surprising and more robust to compile the split_regex with NGX_REGEX_MULTILINE so that the expected regex matched regardless of the character contents of the URL suffix.

Change History (2)

comment:1 by Maxim Dounin, 21 months ago

Resolution: wontfix
Status: newclosed
Type: defectenhancement

In the multiline mode, ^ and $ are going to match on internal newlines. In the particular example, this will truncate path info to just /<CR>, making things worse than non-matching, which is immediately obvious. Further, in some configurations this might make it possible to request arbitrary php files (regardless of location prefix checks) by providing a "hidden" script name after an LF. I don't think it's a good idea.

Single line mode (PCRE_DOTALL) might be a better option, though I don't think I like the idea of using different regex options for different regular expressions within nginx. In particular, it is not clear why fastcgi_split_path_info should be single line, but not location, if, and rewrite. Rather, I would recommend enabling single line mode explicitly with (?s) in the regular expression if needed.

comment:2 by kohler@…, 21 months ago

Thanks for the explanation!

(It might be nice to update the documentation to suggest (?s) or use [\s\S]*, as in fastcgi_split_path_info (?s)^(.+?\.php)(/.*)$; or fastcgi_split_path_info ^(.+?\.php)(/[\s\S]*)$;.)

Note: See TracTickets for help on using tickets.