HTML shown on forbidden and surge protection pages

matthewhughes934 commented 4 years ago

Behaviour

I noticed while browsing https://wiki.debian.org/ (powered by MoinMoin) that <p> tags would be displayed in the message for pages which are Forbidden (i.e. 403 response). Sample MoinMoin response (testing on local by blacklisting my ip):

    $ ./wikiserver.py &>/dev/null &
    $ curl localhost:8080
    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
    <title>403 Forbidden</title>
    <h1>Forbidden</h1>
    <p>&lt;p&gt;You are not allowed to access this!&lt;/p&gt;</p>

Note the escaped tags in the last line. When I started browsing the code it looks like there's a similar issue for the SurgeProtection page (tested locally by just forcing this to be raised on each page):

    $ curl localhost:8080
    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2 Final//EN">
    <title>503 Surge protection</title>
    <h1>Surge protection</h1>
    <p>&lt;strong&gt;Warning:&lt;/strong&gt;&lt;p&gt;You triggered the wiki's surge protection by doing too many requests in a short time.&lt;/p&gt;&lt;p&gt;Please make a short break reading the stuff you already got.&lt;/p&gt;&lt;p&gt;When you restart doing requests AFTER that, slow down or you might get locked out for a longer time!&lt;/p&gt;</p>

Cause

This appears to be caused by MoinMoin.Support.werkzeug.exceptions.HTTPException.get_description which escapes the exception's escape string. Note that this also wraps the description in <p></p> tags itself.

Proposed Solution

Assuming this needs addressing, the easiest solution I see is to simply strip all tags from each exception's description. If this is acceptable I'd happily get a PR up for it

ThomasWaldmann commented 4 years ago

I am in progress of preparing a new moin release with werkzeug 1.0.1 - guess this should be tested after that change, whether it still happens.

ThomasWaldmann commented 4 years ago

How can I reproduce this?

I put some ACLs on a page, then viewed it (-> not allowed).

But I get way more html than you also also i don't see these html tag issues.

matthewhughes934 commented 4 years ago

How can I reproduce this?

Hi, sorry should've included that in the issue. Let me dig up what I had.

matthewhughes934 commented 4 years ago

I was just manually blacklisting my local IP:

diff --git a/MoinMoin/config/multiconfig.py b/MoinMoin/config/multiconfig.py
index 0f03cb50..3f21fa4e 100644
--- a/MoinMoin/config/multiconfig.py
+++ b/MoinMoin/config/multiconfig.py
@@ -851,7 +851,7 @@ options_no_group_name = {
   'spam_leech_dos': ('Anti-Spam/Leech/DOS',
   'These settings help limiting ressource usage and avoiding abuse.',
   (
-    ('hosts_deny', [], "List of denied IPs; if an IP ends with a dot, it denies a whole subnet (class A, B or C)"),
+    ('hosts_deny', ["127.0.0.1"], "List of denied IPs; if an IP ends with a dot, it denies a whole subnet (class A, B or C)"),
     ('surge_action_limits',
      {# allow max. <count> <action> requests per <dt> secs
         # action: (count, dt)

Then just:


$  python wikiserver.py &
$ curl localhost:8080

matthewhughes934 commented 4 years ago

And a very direct approach to force surge detection:

diff --git a/MoinMoin/web/utils.py b/MoinMoin/web/utils.py
index bc18e47f..7e5433cf 100644
--- a/MoinMoin/web/utils.py
+++ b/MoinMoin/web/utils.py
@@ -52,6 +52,7 @@ def check_surge_protect(request, kick=False, action=None, username=None):
     @param action: specify the action explicitly (default: request.action)
     @param username: give username (for action == 'auth-name')
     """
+    raise SurgeProtection(retry_after=60)
     limits = request.cfg.surge_action_limits
     if not limits:
         return False

ThomasWaldmann commented 4 years ago

Ah, ok, that both is in an earlier phase of request processing.

ThomasWaldmann commented 4 years ago

@matthewhughes934 please review / test #67.

as i had to look at the stuff anyway, i continued to just fix it. but you can help be reviewing it, testing it.

moinwiki / moin-1.9

HTML shown on forbidden and surge protection pages #60