git: change calling convention of safe_git to have named arguments

(cherry picked from commit 7f2235478d4331b8738e9a9dc8d6d6c08461cd5c)

git: Do the revert operation in a secondary working tree

This avoids leaving the git directory in an inconsistent state if the
host system is rebooted while we are processing a revert.

(cherry picked from commit 7e84a1f9d88a1f546188a28524fedbdf53bb8729)

git: Turn $git_dir into a stack

This will be necessary when we use a secondary working tree to do
reverts without leaving the primary working tree in an inconsistent
state.

(cherry picked from commit 39b8931ad31fe6b48afdc570caa459a0996c2092)

Add automated test for using the CGI with git, including CVE-2016-10026

(cherry picked from commit fa64672d40f877f3bf9cf245cda0cc3f3837c50c)

Try revert operations (on a branch) before approving them

Otherwise, we have a time-of-check/time-of-use vulnerability:
rcs_preprevert previously looked at what changed in the commit we are
reverting, not at what would result from reverting it now. In
particular, if some files were renamed since the commit we are
reverting, a revert of changes that were within the designated
subdirectory and allowed by check_canchange() might now affect
files that are outside the designated subdirectory or disallowed
by check_canchange().

OVE-20161226-0002

git: do not fail to commit if committer is anonymous

(cherry picked from commit c86046090e1dc31035e4db12e4f29562634d621e)

git: don't issue a warning if rcsinfo is undefined

The intention here seems to be that $prev may be undefined, and the
only way that can legitimately happen is for $params{token} to be
undefined too.

(cherry picked from commit fd14cd2a4355684951bb40a1e72e8b0960a674fd)

Use git log --no-renames for recentchanges

Otherwise, recent git releases show renames as renames, and we do not
see that newdir/test5 was affected.

Bug-Debian: https://bugs.debian.org/835612
(cherry picked from commit 276f0cf57861418fae5b4db8446d3d1098130cdf)

Correctly handle filenames starting with a dash in add/rm/mv.

(cherry picked from commit bbdba8d770b73bc44f55219615b360484b7d240f)

ensure_committer: don't do anything if we have the environment variables

(cherry picked from commit 1f635c6dcaeff8f869f874f659da875c4e7f1863)

Don't memoize ensure_committer

This makes it harder to test, and if we're invoking git anyway,
a couple of extra subprocesses are no big deal.

(cherry picked from commit 8550c397016bd66095f24de64b077526e08bbab2)

git: if no committer identity is known, set it to "IkiWiki <ikiwiki.info>" in .git/config

This resolves commit errors in versions of git that require a non-trivial
committer identity.

(cherry picked from commit ed1e1ebe70c8aec06a759d8cd0168f242d28ac17)

Revert "Tell `git revert` not to follow renames (CVE-2016-10026)"

This doesn't work prior to git 2.8: `git revert` silently ignores the
option and succeeds. We will have to fix CVE-2016-10026 some other way.

This reverts commit bb5cf4a0940b8fd2750c6175adb15382b84c71e2.

Tell `git revert` not to follow renames (CVE-2016-10026)

Otherwise, we have an authorization bypass vulnerability: rcs_preprevert
looks at what changed in the commit we are reverting, not at what would
result from reverting it now. In particular, if some files were renamed
since the commit we are reverting, a revert of changes that were within
the designated subdirectory and allowed by check_canchange() might now
affect files that are outside the designated subdirectory or disallowed
by check_canchange().

Signed-off-by: Simon McVittie <smcv@debian.org>

Update changelog for img backport

img test: exercise upper-case extensions for image files

(cherry picked from commit 6879e3782854f336bb2aa192a36a049695953245)

Detect image type from .JPG just like .jpg (etc.).

(cherry picked from commit 89af9ecc575c3e7223b0e6bea57f82872f5caa9f)

Update changelog

CGI, attachment, passwordauth: harden against repeated parameters

These instances of code similar to OVE-20170111-0001 are not believed
to be exploitable, because defined(), length(), setpassword(),
userinfo_set() and the binary "." operator all have prototypes that
force the relevant argument to be evaluated in scalar context. However,
using a safer idiom makes mistakes less likely.

Force CGI::FormBuilder->field to scalar context where necessary

CGI::FormBuilder->field has behaviour similar to the CGI.pm misfeature
we avoided in f4ec7b0. Force it into scalar context where it is used
in an argument list.

This prevents two (relatively minor) commit metadata forgery
vulnerabilities:

* In the comments plugin, an attacker who was able to post a comment
  could give it a user-specified author and author-URL even if the wiki
  configuration did not allow for that, by crafting multiple values
  to other fields.
* In the editpage plugin, an attacker who was able to edit a page
  could potentially forge commit authorship by crafting multiple values
  for the rcsinfo field.

The remaining plugins changed in this commit appear to have been
protected by use of explicit scalar prototypes for the called functions,
but have been changed anyway to make them more obviously correct.
In particular, checkpassword() in passwordauth has a known prototype,
so an attacker cannot trick it into treating multiple values of the
name field as being the username, password and field to check for.

OVE-20161226-0001

(cherry picked from commit c1120bbbe8fdea20cf64fa12247f4f4a4006c834)

passwordauth: avoid userinfo forgery via repeated email parameter

OVE-20170111-0001

t/passwordauth.t: new automated test for passwordauth

In particular this includes an exploit for OVE-20170111-0001.

passwordauth: prevent authentication bypass via multiple name parameters

Calling CGI::FormBuilder::field with a name argument in list context
returns zero or more user-specified values of the named field, even
if that field was not declared as supporting multiple values.
Passing the result of field as a function parameter counts as list
context. This is the same bad behaviour that is now discouraged
for CGI::param.

In this case we pass the multiple values to CGI::Session::param.
That accessor has six possible calling conventions, of which four are
documented. If an attacker passes (2*n + 1) values for the 'name'
field, for example name=a&name=b&name=c, we end up in one of the
undocumented calling conventions for param:

    # equivalent to: (name => 'a', b => 'c')
    $session->param('name', 'a', 'b', 'c')

and the 'b' session parameter is unexpectedly set to an
attacker-specified value.

In particular, if an attacker "bob" specifies
name=bob&name=name&name=alice, then authentication is carried out
for "bob" but the CGI::Session ends up containing {name => 'alice'},
an authentication bypass vulnerability.

This vulnerability is tracked as OVE-20170111-0001.

Reference CVE-2016-4561 in 3.20141016.3 changelog

3.20141016.3 (for jessie-security)

Do not recommend mimetype(image/*)

Not all image file types are safe for general use: in particular,
image/svg+xml is known to be vulnerable to CVE-2016-3714 under some
ImageMagick configurations.

Document the security fixes in this release

add more details of CVE-2015-2793

update for recent XSS

img: make img_allowed_formats case-insensitive

update test suite for svg passthrough by img directive

Remove build dependency libmagickcore-6.q16-2-extra which was only there
for this test.

img: Add back support for SVG images, bypassing ImageMagick and simply passing the SVG through to the browser

SVG scaling by img directives has subtly changed; where before size=wxh
would preserve aspect ratio, this cannot be done when passing them through
and so specifying both a width and height can change the SVG's aspect
ratio.

(This patch looks significantly more complex than it was, because a large
block of code had to be indented.)

[smcv: drop trailing whitespace, fix some spelling]

Changelog

img: check magic number before giving common formats to ImageMagick

This mitigates CVE-2016-3714 and similar vulnerabilities by
avoiding passing obviously-wrong input to ImageMagick decoders.

img: restrict to JPEG, PNG and GIF images by default

This mitigates CVE-2016-3714. Wiki administrators who know that they
have prevented arbitrary code execution via other formats can re-enable
the other formats if desired.

Update structure of img test from master

Originally from commit cdfb4ab "Run autopkgtest tests using autodep8 and
the pkg-perl team's infrastructure", cherry-picked here to be able to
apply subsequent test coverage extensions in this test.

img test: set old timestamp on source file that will change

This is so that the test will pass even if it takes less than 1 second.

img test: skip testing PDFs if unsupported

img test: use the right filenames when testing that deletion occurs

Also use a less misleading name for the sample SVG: it is no longer empty.
Since commit 105f285a it has contained a blue square.

img: force common Web formats to be interpreted according to extension

A site administrator might unwisely set allowed_attachments to
something like '*.jpg or *.png'; if they do, an attacker could attach,
for example, a SVG file named attachment.jpg.

This mitigates CVE-2016-3714.

HTML-escape error messages (OVE-20160505-0012)

The instance in cgierror() is a potential cross-site scripting attack,
because an attacker could conceivably cause some module to raise an
exception that includes attacker-supplied HTML in its message, for
example via a crafted filename. (OVE-20160505-0012)

The instances in preprocess() is just correctness. It is not a
cross-site scripting attack, because an attacker could equally well
write the desired HTML themselves; the sanitize hook is what
protects us from cross-site scripting here.

img: stop ImageMagick trying to be clever if filenames contain a colon

$im->Read() takes a filename-like argument with several sets of special
syntax. Most of the possible metacharacters are escaped by the
default `wiki_file_chars` (and in any case not particularly disruptive),
but the colon ":" is not.

It seems the way to force ImageMagick to treat colons within the
filename as literal is to prepend a colon, so do that.

Release 3.20141016.2

Fix XSS in openid selector. Thanks, Raghav Bisht.

Conflicts:
debian/changelog
doc/bugs/XSS_Alert...__33____33____33__.html

note that the two potential FTBFSs do not actually affect buildds

The relevant tests are skipped on buildds because they don't have
libipc-run-perl or inkscape.

release candidate

Work around Debian #771047: use a non-blank SVG for the regression test

Inkscape loses the bounding box of a SVG with no content when it
converts it to EPS, and ImageMagick does not have a special case for
converting SVG to PNG with Inkscape in one step (which Inkscape can do);
it prefers to convert SVG to EPS with Inkscape, then EPS to whatever.

close debian bug I opened about blogspam

blogspam uses JSON instead of RPC::XML now.

Update blogspam to the 2.0 API.

Set Debian package maintainer to Simon McVittie as I'm retiring from Debian.

Add missing build-depends on libcgi-formbuilder-perl, needed for t/relativity.t

Merge remote-tracking branch 'refs/remotes/dgit/dgit/sid'

release

debian: fix some wrong paths in the copyright file

debian: rename debian/link to debian/links so the intended symlinks appear

close a bug

Drop unused python-support dependency

changelog so far

build-depend on libcgi-pm-perl too, for tests

Explicitly depend on CGI.pm, which is no longer in Perl core

I was going to depend on the version that has CGI->param_fetch,
but that has been supported since 2.37, which is older than oldstable.

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.

[commit message added by smcv]

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.

[commit message added by smcv]

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.

Added a comment: It was an Apache problem...

branch

comment

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>).

mention pagespec_alias patches

Added a comment

Added a comment

Added a comment

Added a comment

Added a comment

as usual, macports hasn't moved

Added a comment

Added a comment

one report suffices; not yet clear there's a bug

clarify

findings and questions

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.

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.

google search plugin: use https for the search

default User-Agent changed

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.

removed

Added a comment

help Markdown make a list

Added a comment: fixed in a recent release, I think

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.

Extract test subs for each site. No change meant.

Extract run_cgi(). No functional change intended.

Extract check_generated_content(). Same output.

Extract check_cgi_mode_bits(). No change intended.

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.