apache / incubator-pagespeed-mod

Apache module for rewriting web pages to reduce latency and bandwidth.
http://modpagespeed.com
Apache License 2.0
696 stars 159 forks source link

mod_pagespeed File name too long #1648

Open bararchy opened 6 years ago

bararchy commented 6 years ago

While fuzzing a website for testing I noticed that pagespeed will create long and elaborate files which it then cannot read back: screenshot at 15-42-31

Not sure if related but the cache folder looks like this:

screenshot at 15-46-33

jmarantz commented 6 years ago

I suspect nothing really disastrous happens, except that time is wasted attempting to write cache files that can never be read. That is legal though suboptimal behavior for a cache. That is, the system should continue to serve valid web pages, though it may waste CPU attempting to optimize resources that will never be served. And it will also fill logs with messages like you pasted, and that may compromise server stability over time as the disks fill with log messages.

I think we can clean this up by dropping attempts to optimize excessively long URLs.

bararchy commented 6 years ago

@jmarantz If it can't read the files, can it clean them ? is there a file name limit when calling remove ?

jmarantz commented 6 years ago

I think the error message you are seeing is actually for writing, so it won't succeed in writing them. So cleaning them is not going to be needed.

Nice job on the fuzzing by the way. It probably is a good idea to fix this. One more general thought on how to fix -- but there's a place in the system where a URL is checked against the Disallow FastWildcardGroup in RewriteOptions. In that place we should see also whether the URL is too long.

In fact there already is this documented limit: https://www.modpagespeed.com/doc/restricting_urls#segment_length -- did you override that? and there is an undocumented one: MaxUrlSize (with a TODO to document...).

bararchy commented 6 years ago

Nice job on the fuzzing by the way.

Thanks :) I have used our fuzzer called Nexploit, which uses machine learning to perform fuzzing . Are you using any fuzzers as part of your work on this project ?

In fact there already is this documented limit: modpagespeed.com/doc/restricting_urls#segment_length -- did you override that?

I got #ModPagespeedMaxSegmentLength 250 commented out, looking at the docs this means it defaults to 1024 right ? Should I retest using 250 as a limiter ?

I didn't play around with the configurations, I'm using an out of the box OVA from bitnami for Wordpress which uses those settings, so I guess it's a common default.

bararchy commented 6 years ago

@jmarantz I took the liberty of re-testing using the 250 settings, this issue still happens

screenshot at 10-39-43

would you like me to pass you a script that can re-produce the issue ?

jmarantz commented 6 years ago

I looked more carefully at the error message and this is generated on an html request. I think we are not applying the URL length limits in this context and probably should be. It should not be hard to reproduce.

Question: is this issue blocking you in some way? Or is It just something you noticed while fuzzing? Note that other than log spam, there is no incorrect behavior here afaict.

bararchy commented 6 years ago

@jmarantz not blocking and not high priority from my side , was just part of the ongoing fuzzing I did on apache (and found this on the way) , just wanted to help track it down .