Amitai Schlair [Wed, 15 Oct 2014 22:52:43 +0000 (23:52 +0100)]
IkiWiki::Plugin::openid: as a precaution, do not call non-coderefs
We're running under "use strict" here, so if CGI->param's array-context
misbehaviour passes an extra non-ref parameter, it shouldn't be executed
anyway... but it's as well to be safe.
Amitai Schlair [Wed, 15 Oct 2014 21:32:02 +0000 (22:32 +0100)]
Call CGI->param_fetch instead of CGI->param in array context
CGI->param has the misfeature that it is context-sensitive, and in
particular can expand to more than one scalar in function calls.
This led to a security vulnerability in Bugzilla, and recent versions
of CGI.pm will warn when it is used in this way.
In the situations where we do want to cope with more than one parameter
of the same name, CGI->param_fetch (which always returns an
array-reference) makes the intention clearer.
Simon McVittie [Sat, 11 Oct 2014 08:28:22 +0000 (09:28 +0100)]
Make sure we do not pass multiple CGI parameters in function calls
When CGI->param is called in list context, such as in function
parameters, it expands to all the potentially multiple values
of the parameter: for instance, if we parse query string a=b&a=c&d=e
and call func($cgi->param('a')), that's equivalent to func('b', 'c').
Most of the functions we're calling do not expect that.
I do not believe this is an exploitable security vulnerability in
ikiwiki, but it was exploitable in Bugzilla.
Simon McVittie [Wed, 15 Oct 2014 20:56:11 +0000 (21:56 +0100)]
Replace PayPal and Flattr buttons with text links
In particular, this avoids loading third-party resources from the
offline documentation (see
<https://lintian.debian.org/tags/privacy-breach-donation.html>).
Simon McVittie [Sat, 11 Oct 2014 08:28:02 +0000 (09:28 +0100)]
Do not pass ignored sid parameter to checksessionexpiry
checksessionexpiry's signature changed from
(CGI::Session, CGI->param('sid')) to (CGI, CGI::Session) in commit 985b229b, but editpage still passed the sid as a useless third
parameter, and this was later cargo-culted into remove, rename and
recentchanges.
Simon McVittie [Sun, 12 Oct 2014 17:03:28 +0000 (18:03 +0100)]
comments: don't log remote IP address for signed-in users
The intention was that signed-in users (for instance via httpauth,
passwordauth or openid) are already adequately identified, but
there's nothing to indicate who an anonymous commenter is unless
their IP address is recorded.
Simon McVittie [Sat, 11 Oct 2014 08:43:34 +0000 (09:43 +0100)]
Set default User-Agent to something that doesn't mention libwww-perl
It appears that both the open-source and proprietary rulesets for
ModSecurity default to blacklisting requests that say they are
from libwww-perl, presumably because some script kiddies use libwww-perl
and are too inept to set a User-Agent that is "too big to blacklist",
like Chrome or the iPhone browser or something. This seems doomed to
failure but whatever.
Amitai Schlair [Sun, 12 Oct 2014 15:08:13 +0000 (11:08 -0400)]
Replace shebang paths with the build-time $(PERL).
On non-Debian systems, /usr/bin/perl might not be the best available
Perl interpreter. Use whichever perl was used to run Makefile.PL,
unless it was "/usr/bin/perl", in which case there's nothing to do.
Amitai Schlair [Sat, 11 Oct 2014 00:40:24 +0000 (20:40 -0400)]
Extract thoroughly_rebuild(), a slight test change.
I didn't try to parameterize when a test should fail when we can't
remove ikiwiki.cgi because there already isn't one. (Hooray, natural
language.) Instead, we stop worrying about it and always tolerate
ENOENT.
Fix crash that can occur when only_committed_changes is set and a file is deleted from the underlay.
srcfile_stat got called on a file from the underlay that no longer existed.
I am not 100% sure of the circumstances of that; I was able to reproduce
the bug but neglected to snapshot the tree, and then accidentially
got it to stop crashing. I know that a transient tag page got deleted using
the web interface to trigger the crash.
It seems that process_changed_files must have returned the file, despite it
being deleted. And since the file was not checked into git, it seems it
must have not been included in @IkiWiki::underlayfiles, which would have
caused process_changed_files to not return it.
I do not know why a transient tag page would not be in
@IkiWiki::underlayfiles. There is a bug here that I don't understand.
This is just a workaround -- run srcfile_stat such that it won't crash,
and if it is unable to stat a file, find_changed knows it's not changed,
so it's ok to skip it.
Also made find_new_files run srcfile_stat such that it won't crash, just
because I was there.