Page MenuHomePhabricator

mw-config/w/static.php should not serve dotfiles ever [paranoia]
Closed, ResolvedPublic

Description

dotfiles often contain sensitive things (e.g. .git directory, or a editor swap files, temp files). We will serve them in the extension directory if they have a safe sounding file extension. For example https://en.wikipedia.org/w/extensions/WikimediaMaintenance/.phpcs.xml

While I don't think we're vulnerable in any way (We don't deploy .git files.), I think this is a good hardening step.

Fun fact: Under certain obscure circumstances (A lot of other potential file names taken), vim may use .svg as a file extension for its swap file!

Event Timeline

Additionally, given:

                $filePath = realpath( "$branchDir/$path" );
                if ( !$filePath ) { 
                        continue;
                }   

                if ( strpos( $filePath, $branchDir ) !== 0 ) { 
                        wmfStaticShowError( 'Bad request', 400 );
                        return;
                }
[...]
                wmfStaticShowError( 'Unknown file path', 404, 300 );
                $stats->increment( 'wmfstatic.notfound' );
                return;

Having the error be different for path traversal attempt where the file is found vs path traversal attempt where the file is not found might not be great (as it can expose what files are on the system). In practise this seems pretty impossible to trigger as some other layer (presumably varnish) normalizes paths before they get to this script. [And in our setup, what files are where on the system isn't exactly super secret]

sbassett moved this task from Backlog / Other to Done on the acl*security board.
sbassett subscribed.

@Krinkle pushed through gerrit and deployed. Resolving and making public.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett awarded a token.