Conversation

twitter.com/DanielMicay/st Most of these security issues are indirectly resolved in a stable nginx release with 1.22.0. Most fixes are from the 1.21.1 mainline release in July 2021. However, they weren't marked or treated as security fixes so the 1.20 branch didn't get backports.

Quote Tweet

I love how using `return $uri` instead of `return $request_uri` is a vulnerability for nginx configuration since they don't sanitize the input and allow it to inject data into the headers via newlines. Alternatively, capturing/using any variables in location blocks with newlines.

Show this thread

2

3

8

The configuration directives are still vulnerable and can still be easily used in a way that allows an attacker to inject headers for HTTP/1.1. Their fixes forbid spaces and control characters in request URI, header names and Host header so most sources of tainted data are gone.

1

Previously, incredibly pervasive nginx configurations simply using a regex location block with a capture like (.*) and then passing it to one of many directives like return were vulnerable to header injection. Many answers on Stack Overflow, blog posts, etc. have vulnerable code.

1

You can find many examples of Stack Overflow answers proposing vulnerable configurations with simple searches: "return 301 $uri" site:stackoverflow.com These vulnerabilities are pervasive in nginx configurations and they strangely don't mention this in any official docs.

2

2

@winter_queue_t

Replying to

So imo it's insane that this even needed to be developed, but Yandex made a static analysis tool for Nginx configurations, and one of its rules spots and warns against this:

gixy/httpsplitting.md at master · yandex/gixy

Nginx configuration static analyzer. Contribute to yandex/gixy development by creating an account on GitHub.

1

Replying to

@winter_queue_t

Yeah, we use it for GrapheneOS. It's unfortunately not maintained anymore and bitrotted due to a major pyparsing update. It doesn't work anymore unless you force holding back the update. It'll be a bit annoying to use now that the main tainted input sources have strict checks.

1

Replying to

and

@winter_queue_t

It's unfortunate directives like return are still vulnerable. Most vulnerable configuration will be fixed by people updating to 1.21 / 1.22 but there will still be plenty of vulnerabilities. It's unfortunate they don't even mention those directives don't do escaping in docs.

1

@winter_queue_t

Replying to

> It'll be a bit annoying to use now that the main tainted input sources have strict checks. What do you mean, how does that make Gixy more annoying to use? Do you mean that it would now have false positives due to better Nginx behavior?

2

@winter_queue_t

Replying to

@winter_queue_t

and

> It's unfortunate they don't even mention those directives don't do escaping in docs. It's also unfortunate that things like "if in location is evil" are only mentioned on nginx.com, and not the main documentation on nginx.org.

Advanced Load Balancer, Web Server, & Reverse Proxy - NGINX

NGINX accelerates content and application delivery, improves security, and facilitates availability and scalability for the busiest websites on the Internet.

1

Replying to

@winter_queue_t

Their statements about the if directive are problematic because they should be telling people what's safe and what isn't separately from documenting best practices for performance / style. It's fine to use it if it only contains return/rewrite directive. Other stuff is broken.

3:40 AM · Jun 3, 2022Twitter Web App

Replying to

and

@winter_queue_t

It's unfortunate they started permitting using other directives while not actually supporting it or fixing all the terrible issues occurring when you do. It's fine inside location blocks as long as you follow that rule and it's broken outside location blocks too if you don't.

1

Replying to

and

@winter_queue_t

For example, their documentation on that talks about how you should use server blocks, location blocks, try_files, etc. instead of if directives for better performance, readability and ease of doing things correctly. Often you do need if directives to do things properly though.

1

Show replies

@winter_queue_t

Replying to

Their best course of action would probably be to outright refuse to load a configuration that uses the unsafe directives in that context, honestly. But who knows if they'd actually do that. :/

1

Replying to

@winter_queue_t

There are a few examples of completely correct and sensible usage of if directives in github.com/GrapheneOS/gra and our other nginx configurations. #1 is safely removing double slashes in a single redirect which is important for SEO to avoid having multiple locations for content.

grapheneos.org/nginx.conf at main · GrapheneOS/grapheneos.org

Main website servers. Contribute to GrapheneOS/grapheneos.org development by creating an account on GitHub.

1

Show replies