Commit Graph

3343 Commits (54f6ca83c57d53e537bc82738eafff0cbe074097)

Author SHA1 Message Date
Simon McVittie 54095ea655 img: Explicitly detect format if needed
An empty coder name used to detect the format implicitly, but has been
interpreted as a literal part of the filename since ImageMagick 6.9.8-3.
In newer versions, there does not seem to be any way to indicate that
a filename containing ':' is to be taken literally without first
knowing the decoder to use.

Signed-off-by: Simon McVittie <smcv@debian.org>
2018-02-28 09:56:08 +00:00
Simon McVittie f46e429d96 mdwn: Restore historical behaviour
The Discount package in Debian historically enabled fenced code blocks,
PHP Markdown Extra-style definition lists, and an expanded character
set for tag names. Since Discount 2.2.0 those are runtime settings, so
enable them. Unfortunately Text::Markdown::Discount doesn't yet expose
the necessary constants:
https://rt.cpan.org/Public/Bug/Display.html?id=124188

The IDANCHOR option was historically also enabled in Debian, but is not
enabled here because ikiwiki does not enable the TOC option, and
IDANCHOR does nothing without TOC.

Closes: #888055
2018-01-29 22:35:29 +00:00
Simon McVittie a147f5349d Don't send relative redirect URLs when behind a reverse proxy 2018-01-08 10:56:12 +00:00
Joey Hess a79ab9ed18
add and use cgiurl_abs_samescheme
* emailauth: Fix cookie problem when user is on https and the cgiurl
   uses http, by making the emailed login link use https.
 * passwordauth: Use https for emailed password reset link when user
   is on https.

Not entirely happy with this approach, but I don't currently see a
better one.

I have not verified that the passwordauth change fixes any problem,
other than the user getting a http link when they were using https.
The emailauth problem is verified fixed by this commit.

This commit was sponsored by Michael Magin.
2018-01-05 11:59:35 -04:00
Joey Hess 61f0dc669f
improve error message when postsignin is not set
This can happen if the user goes directly to /ikiwiki.cgi?do=login and
logs in, since nothing redirected them to there, there's no postsignin
value set. It can also happen when cookies are disabled, or perhaps
other problems.
2018-01-04 19:21:51 -04:00
Simon McVittie 6fb43c29f6 Pass some git environment variables to the test-receive hook
Since git 2.11, git has stored the proposed push in a "quarantine
area" until it is accepted by the pre-receive hook, and passed
extra environment variables to the pre-receive hook so that it can
read objects from the quarantine area.

This fixes untrusted push on modern git versions.

Signed-off-by: Simon McVittie <smcv@debian.org>
2017-10-01 13:21:56 +01:00
Simon McVittie cf7df018cc IkiWiki::Receive: Avoid using asprintf
On GNU/Linux, it isn't declared in stdio.h unless we define
_GNU_SOURCE, which we don't; using the implicit declaration risks
crashes on platforms where sizeof(pointer) != sizeof(int). On other
platforms it isn't guaranteed to exist at all.

Signed-off-by: Simon McVittie <smcv@debian.org>
2017-10-01 13:21:56 +01:00
intrigeri 146f1d9ab5 img: resize images in a deterministic manner. 2017-09-01 19:26:26 +00:00
Joey Hess e3dfb26b90
emailauth, passwordauth: Avoid leaving cgisess_* files in the system temp directory.
Due to the use/abuse of CGI::Session to generate a token for the login
process, a new session database was created for each login, and left behind
afterwards. While each file is small, with many logings this could bloat
the size of /tmp significantly. Fixed by making CGI::Session write to
/dev/null, since there does not seem to be a way to entirely prevent the
writing.

This commit was sponsored by Henrik Riomar on Patreon.
2017-08-23 13:13:23 -04:00
Joey Hess f3beb9cb44
htmlscrubber: Add support for the video tag's loop and muted attributes.
Those were not in the original html5 spec, but have been added in the
whatwg html living standard and have wide browser support.

This commit was sponsored by John Peloquin on Patreon.
2017-07-11 15:51:44 -04:00
Simon McVittie 5a84cd308d osm: Convert savestate hook into a changes hook
savestate is not the right place to write wiki content, and in particular
this breaks websetup if osm's dependencies are not installed, even if
the osm plugin is not actually enabled. (Closes: #719913)

This is not a full solution: it should be possible to render the PoI files
for only the maps that changed, from the format, changes or rendered
hook. However, getting that right would require more understanding of
this plugin, and this version is enough to not break websetup. This
version is the closest correct hook to the one where this previously
took place.
2017-06-20 23:47:08 +01:00
Simon McVittie 01f2a84360 color: Use markup for the preserved CSS, not character data
This still smuggles it past the sanitize step, but avoids having
other plugins that want to capture text content without markup
(notably toc) see the CSS as if it was text content.
2017-05-16 12:08:55 +01:00
Antoine Beaupré b5b48d5bb5 use heading identifiers in TOC links
reasoning: if headings have identifiers, they are probably more useful
anchors than the automatically generated anchors we build in the toc
plugin. this can happen if, for example, you use the `multimarkdown`
plugin, which inserts `id` tags for every header it encounters. this
also leverages the `headinganchors` plugin nicely.

keeps backwards-compatibility with old toc-generated #indexXhY
anchors.
2017-05-16 08:55:23 +01:00
Simon McVittie c72dc5ddb7 mdwn: Don't enable alphabetically labelled ordered lists by default
This avoids misinterpreting initials ("C. S. Lewis was an author"),
the abbreviation for Monsieur ("M. Descartes was a philosopher") and
German page numbering ("S. 42") as ordered lists if they happen to
begin a line.

This only affects the default Discount implementation: Text::Markdown
and Text::MultiMarkdown do not have this feature anyway. A new
mdwn_alpha_list option can be used to restore the old interpretation.
2017-05-16 08:09:15 +01:00
Simon McVittie 4db4e589e4 mdwn: Enable footnotes by default when using Discount
A new mdwn_footnotes option can be used to disable footnotes in
MultiMarkdown and Discount.
2017-05-14 18:16:53 +01:00
Simon McVittie 81c3258269 mdwn: Don't mangle <style> into <elyts> under some circumstances
We can ask libdiscount not to elide <style> blocks, which means we
don't have to work around them.
2017-05-14 17:45:55 +01:00
Simon McVittie 5d65c1ad2c mdwn: Make flags passed to libdiscount explicit
The Perl binding defaults to MKD_NOHEADER|MKD_NOPANTS anyway, but
making them explicit means we can use other flags of our choice,
and makes it easier to justify why those flags are appropriate.
2017-05-14 17:34:12 +01:00
Simon McVittie 60cb2ac458 cgierror: When the CGI fails, print the error to stderr, not "Died"
$@ could be clobbered by the "exception handler", and in practice
it seems that it is. This can be seen on stderr of t/git-cgi.t.
2017-05-14 15:39:21 +01:00
Simon McVittie 26ded17653 Defend against empty session names
If misconfiguration has resulted in an empty session name, treat the
session as having not signed in.
2017-05-14 15:37:47 +01:00
Simon McVittie 31c89db246 httpauth: If REMOTE_USER is empty, behave as though it was unset
A frequently cut-and-pasted HTTP basic authentication configuration
for nginx sets it to the empty string when not authenticated, which
is not useful.
2017-05-14 15:37:45 +01:00
Simon McVittie 2486d83706 remove: make it clearer that repeated page parameter is OK here
ikiwiki's web interface does not currently have UI for removing
multiple pages simultaneously, but the remove plugin is robust
against doing so. Use a clearer idiom to make that obvious.
2017-01-11 18:11:21 +00:00
Simon McVittie d157a97452 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.

(cherry picked from commit 69230a2220f673c66b5ab875bfc759b32a241c0d)
2017-01-11 18:11:07 +00:00
Simon McVittie b642cbef80 passwordauth: avoid userinfo forgery via repeated email parameter
OVE-20170111-0001

(cherry picked from commit bffb71d6a7d28f6dd5f0be241f214e79eea7bb91)
2017-01-11 18:11:07 +00:00
Simon McVittie f357856448 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.

(cherry picked from commit e909eb93f4530a175d622360a8433e833ecf0254)
2017-01-11 18:11:06 +00:00
Simon McVittie 0463357392 git: don't redundantly pass "--" to git_sha1
git_sha1 already puts "--" before its arguments, so

    git_sha1_file($dir, 'doc/index.mdwn')

would have incorrectly invoked

    git rev-list --max-count=1 HEAD -- -- doc/index.mdwn

If there is no file in the wiki named "--", that's harmless, because
it merely names the latest revision in which either "--" or
"doc/index.mdwn" changed. However, it could return incorrect results
if there is somehow a file named "--".
2017-01-09 13:58:58 +00:00
Simon McVittie 59632384d9 git: use parameters, not global state, to swap working directory 2017-01-09 13:50:54 +00:00
Simon McVittie c29c230c33 Revert "git: Turn $git_dir into a stack"
Now that we have avoided using in_git_dir recursively, we don't need
the stack any more.

This reverts commit 39b8931ad3.
2017-01-09 13:07:24 +00:00
Simon McVittie 6504456454 git: do not mix in_git_dir with eval{}
If we throw an exception (usually from run_or_die), in_git_dir won't
unshift the current directory from the stack. That's usually fine,
but in rcs_preprevert we catch exceptions and do some cleanup before
returning, for which we need the git directory to be the root and
not the temporary working tree.
2017-01-09 13:07:24 +00:00
Simon McVittie 9e7f0a6c59 Use rel=nofollow microformat for dynamic (CGI-related) URLs
Some of these might be relatively expensive to dereference or result
in messages being logged, and there's no reason why a search engine
should need to index them. (In particular, we'd probably prefer search
engines to index the rendered page, not its source code.)
2017-01-09 13:07:24 +00:00
Amitai Schleier 8bf2273937 Enquote $background_command as surely intended. 2017-01-02 06:57:35 -05:00
Simon McVittie d092b0b777 git: Do not disable commit hook for temporary working tree
We exclude .git/hooks from symlinking into the temporary working tree,
which avoids the commit hook being run for the temporary branch anyway.
This avoids the wiki not being updated if an orthogonal change is
received in process A, while process B prepares a revert that is
subsequently cancelled.
2016-12-29 20:46:38 +00:00
Simon McVittie afda054796 git: Attribute reverts to the user doing the revert, not the wiki itself 2016-12-29 20:43:15 +00:00
Simon McVittie 4ad4fc33b5 git: write proposed attachment to temp file without going via system() 2016-12-28 21:32:12 +00:00
Simon McVittie 7f2235478d git: change calling convention of safe_git to have named arguments 2016-12-28 21:32:12 +00:00
Simon McVittie 7e84a1f9d8 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.
2016-12-28 21:32:12 +00:00
Simon McVittie 39b8931ad3 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.
2016-12-28 21:32:12 +00:00
Simon McVittie a8a7462382 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().

It is not sufficient to disable rename detection, since git older
than 2.8.0rc0 (in particular the version in Debian stable) silently
accepts and ignores the relevant options.

OVE-20161226-0002
2016-12-28 21:32:12 +00:00
Simon McVittie 469c842fd5 Revert "Tell `git revert` not to follow renames"
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 9cada49ed6.
2016-12-28 21:32:12 +00:00
Simon McVittie c1120bbbe8 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
2016-12-28 21:32:12 +00:00
Simon McVittie e193c75b7d git: do not fail to commit if committer is anonymous 2016-12-28 21:32:12 +00:00
Simon McVittie a67f4d3944 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.
2016-12-28 21:32:12 +00:00
intrigeri c1890c116d Make pagestats output more deterministic.
Sort in lexical order the pages that have the same number of hits.
2016-12-19 18:21:07 +00:00
Simon McVittie 9cada49ed6 Tell `git revert` not to follow renames
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>
2016-12-19 18:21:07 +00:00
Simon McVittie b0b1428e62 cgitemplate: actually remove dead code
Signed-off-by: Simon McVittie <smcv@debian.org>
2016-12-19 13:20:55 +00:00
Joey Hess 68e2320696
inline: Prevent creating a file named ".mdwn" when the postform is submitted with an empty title. 2016-09-21 13:51:42 -04:00
Amitai Schlair 85c10d149b Update my surname to its new legal spelling. 2016-09-14 14:28:01 -04:00
Simon McVittie 276f0cf578 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
2016-09-03 23:47:06 +01:00
Joey Hess d9bfe760d1
improve warning message for multiple sources for page 2016-05-31 15:29:09 -04:00
Simon McVittie 5f6f9a1bea Wrapper: allocate new environment dynamically
Otherwise, if third-party plugins extend newenviron by more than
3 entries, we could overflow the array. It seems unlikely that any
third-party plugin manipulates newenviron in practice, so this
is mostly theoretical. Just in case, I have deliberately avoided
using "i" as the variable name, so that any third-party plugin
that was manipulating newenviron directly will now result in the
wrapper failing to compile.

I have not assumed that realloc(NULL, ...) works as an equivalent of
malloc(...), in case there are still operating systems where that
doesn't work.
2016-05-11 09:18:14 +01:00
Amitai Schlair 89af9ecc57 Detect image type from .JPG just like .jpg (etc.). 2016-05-08 18:31:02 -04:00