Shame it wasn’t caught by any existing test though.
# Seems to have a side affect for any subsequent GET's
This is seemingly a well-intentioned cleanup that misunderstood the branching logic. The original code was normalizing `rc < 0` to COND_RC_NOMATCH (0), but leaving `rc >= 0` as the original value. However the new code accidentally normalizes `rc >= 0` to COND_RC_MATCH (1) while it should be normalizing `rc > 0` to COND_RC_MATCH (leaving 0 as COND_RC_NOMATCH).
There are a few other logic changes in this patch which should likely be reviewed carefully. For example https://github.com/apache/httpd/commit/dd98030cb399e962aa605... does `rc <= COND_RC_MATCH` which matches both match and no match. Presumably it is checking for COND_RC_STATUS_SET but it seems like and odd way to write `rc == COND_RC_STATUS_SET`. Maybe the intention is to match future special values?
If I'm wrong I apologise for reading too much into it but if I'm right please add context.
So, two weeks ago? Meaning, everybody running a version of Apache older than two weeks is safe?
It looks like not even Arch Linux had that version in their repo yet (currently 2.4.63-3) [1]
Main ones: FreeBSD, Alpine, Fedora 42, OpenSUSE Tumbleweed
> If you want to participate in actively developing Apache please subscribe to the dev@httpd.apache.org mailing list as described at https://httpd.apache.org/lists.html#http-dev
The attacks on OpenSSL maintainers ten years ago were disgusting, and I think we've learned nothing since then.
I didn't register the attacks, but I'm sure there were some when you say it.
I summarized the blame on that incident xkcd's wording:
"some random person has been thanklessly maintaining since 2003"
And ASF does receive funding, by the way, even if not much (slightly less than two and a half million USD in 2024).
A quick eyeball of the projects list looks like about 100 projects: https://projects.apache.org/projects.html
So each project gets funded enough for 10% of a developer. That's not enough to provide infrastructure to commercial users satisfaction.
Apache HTTPD still seems to run about 17% of all sites, plenty of those probably make money using the software. https://www.netcraft.com/blog/january-2025-web-server-survey
Open source is open, so naturally people can use it but the ecosystem has also been at a breaking point for years and bad actors has caught the scent of that.
Apache httpd existed many years before junit was invented in 1997. Long before TDD became a thing and our rigorous modern understanding set in. For a second, I even thought the Apache Foundation (founded because of httpd) later hosted junit, but I was wrong, is the Eclipse Foundation.
Some people were writing their own ad-hoc scaffolds before that but it wasn't a widespread practice. Testing meant manual testers clicking on things in the UI, and sophisticated testing was if you had checklists of things to test manually.
Though I agree, that although not a technical justification, an explanation as to why there are no tests is because Apache HTTP is from the 90's. Not writing unit tests was par for course back then. Most FLOSS code bases in the 90s didn't have unit tests, let a alone a CI to run the test suite for each change. Adding tests later is hard. Though there are some tests under the test folder.
COND_RC_MATCH comes from a newly introduced enum `cond_return_type` but `rc` is still declared as int (`int rc = COND_RC_NOMATCH;`).
At least the `rc` from the call to `ap_expr_exec_re` in line 4270 should be an intermediate variable so that `rc` can be defined correctly as the enum type, so that similar mistakes would be flagged as a warning at compile time.
Though this doesn't seem to be a good way of doing that anyway, certainly not on its own (perhaps as a low resource initial test it is valid, in a bloom filter sort of way it could cover some "definitely shouldn't be here" cases efficiently).
RewriteCond expr "! %{HTTP_REFERER} -strmatch '*://%{HTTP_HOST}/*'"
RewriteRule "^/images" "-" [F]The whole feature simply does not work.
Security becomes irrelevant if the whole Apache module is broken.
If you use RewriteCond as the basis of securing your website, that's a security issue for you.
If it's a security issue for a significant number of users, or if the documentation recommends using the directive for a security role, then it's also a security issue for the product itself.
RewriteEngine On
RewriteBase /
RewriteRule ^index\.php$ - [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]
I have to assume that this form of RewriteCond is pretty rare or the bug would have been caught much sooner.
Regarding how quickly it was caught, bugs like this are a clear argument against the idea of "always update immediately" that's pushed down everyone's throat these days, and parroted by so many who don't realise it's part of marketing. Luckily updates do tend to be a lot slower when they're components of managed packages (in this case, WHM and the like).
- This only affects rewrite conditions which literally start with “RewriteCond expr”; this is a special form that causes the condition pattern to be treated as an Apache expression. See the documentation on that feature here: https://httpd.apache.org/docs/trunk/mod/mod_rewrite.html#:~:...
- Yes, there are tests. They’re stored in a separate repository. Here are the regression tests added for this bug: https://github.com/apache/httpd-tests/commit/48a85e34051959c.... As for why testing didn’t find this bug in the first place, you can see that they have tests for RewriteCond, but just not for expression conditions, likely due to the relative rarity of that subfeature.
* There is some crucial counting/reference/condition code that contains a bug
* There is exactly 0 tests for that code
* A fix gets done, but no tests
Coming from dynlangs this does strike me as irresponsible. I believe the previous case I saw was the S3-compatibility header change in ceph, and similar with CORS configuration there too.
Is it so that experienced C developers assume the compiler will flag any bugs that matter?..
The bug appeared to be introduced on July 7 with a backport from trunk:
https://github.com/apache/httpd/blame/ed99ef021de902363c36af...
It took me a second but the fix addresses the case where rc==0. If statement is less than 0. Therefore, rc==0 should indicate no match. https://github.com/apache/httpd/commit/8abb3d06b23975705ebcf...
I suppose because there is also the conditional for err?
binaryturtle•6mo ago