Item1945: CSRF uses relative location for redirect which violates HTTP specs

pencil
Priority: Low
Current State: Closed
Released In: 1.1.0
Target Release: minor
Applies To: Engine
Component: CSRF
Branches:
Reported By: MartinVonGagern
Waiting For:
Last Change By: KennethLavrsen
Foswiki strikeone CSRF validation violates HTTP specs.
  1. It generates a "302 Found" redirect with a relative URL in the Location header. RFC 2616 section 14.30 requires it to be absolute.
  2. It then generates a "419 unused" error page with the confirmation dialog. The status code 419 has been defined in some drafts, like RFC 2616 draft 01 section 10.4.20 or RFC 2518 draft 05 section 10.4, but not in any HTTP standard I know of, and while a comment in the Perl code states that this code was "chosen by foswiki devs", it fails to hint at the reason why it was chosen. For the sake of interoperability, a standard error code like "403 Forbidden" or "409 Conflict" should probably be used instead. The former, as access without a valid key is forbidden, or the latter, as the list of acceptable keys can be considered part of the state of the resource, and as the 409 error page is probably more likely to be displayed to users than being acted on by browsers.

Steps to reproduce:
  1. Visit the Trunk Sandbox (which currently employs strikeone)
  2. Create a new numbered test topic
  3. Start Wireshark or similar to grab network traffic
  4. Disable JavaScript in your Browser
  5. Type some text
  6. Save the topic
  7. Examine captured traffic

You will find this conversation in the captured traffic:
  1. send POST /bin/save/Sandbox/TestTopicAUTOINC0 HTTP/1.1 ...
  2. recv HTTP/1.1 302 Found ... Location: /bin/login/Sandbox/TestTopicAUTOINC0/foswiki_redirect_cache/<some hex> ...
  3. send GET /bin/login/Sandbox/TestTopicAUTOINC0/foswiki_redirect_cache/<some hex> HTTP/1.1 ...
  4. revc HTTP/1.1 419 unused ...

It should be like this instead:
  1. send POST /bin/save/Sandbox/TestTopicAUTOINC0 HTTP/1.1 ...
  2. recv HTTP/1.1 302 Found ... Location: http://trunk.foswiki.org/bin/login/Sandbox/TestTopicAUTOINC0/foswiki_redirect_cache/<some hex> ...
  3. send GET /bin/login/Sandbox/TestTopicAUTOINC0/foswiki_redirect_cache/<some hex> HTTP/1.1 ...
  4. revc HTTP/1.1 409 Conflict ...

-- MartinVonGagern - 19 Aug 2009

The bug report is not acurate. CSRF doesn't violate HTTP specs in any form.

HTTP spec does NOT define 419, which is why it was chosen. None of the codes you suggest are what is really happening, which is why we chose a "new" code. The CSRF confirmation page isn't a conflict, not a security issue (and we cannot use 403 because there is a high probability this would get caught by Apache to send back the Registation page).

We didn't want any installation to catch the HTTP return code we were sending back, as we need this page to arrive intact to the user, otherwise he won't be able to do anything.

So if you want more details on why we chose 419, it's because we needed a new one, and we thought 419 was OK. I can't find back the discussion that took place, but it was most likely on IRC.

Setting to "No action required" as CSRF doesn't violate HTTP specs in any way. RFC 2616 states:
  • HTTP status codes are extensible. HTTP applications are not required to understand the meaning of all registered status codes, though such understanding is obviously desirable. However, applications MUST understand the class of any status code, as indicated by the first digit, and treat any unrecognized response as being equivalent to the x00 status code of that class, with the exception that an unrecognized response MUST NOT be cached. For example, if an unrecognized status code of 431 is received by the client, it can safely assume that there was something wrong with its request and treat the response as if it had received a 400 status code. In such cases, user agents SHOULD present to the user the entity returned with the response, since that entity is likely to include human- readable information which will explain the unusual status.

-- OlivierRaginel - 25 Sep 2009

OK, I now understand your reason for status code 419, and I concede it's not a violation of the spec.

However, that doesn't affect the other half of my bug report, the one about the Location of the redirect being a relative URL. Resetting status to "New" to get that issue addressed.

-- MartinVonGagern - 25 Sep 2009

For the record and for my understanding.

Has the relative location of the redirect any visible effect in any of the known browsers?

I ask not because I want to discard the bug. I just want to re-evaluate if the priority should be low, normal or urgent. If the feature was broken in IE or FF I would change it to urgent. If no known browser has the issue then low is correct.

-- KennethLavrsen - 28 Sep 2009

No visible effect in any browser that I know of, neither IE nor FF. Low priority is perfectly acceptable.

-- MartinVonGagern - 29 Sep 2009

Changed to abs url and commented the use of 419

-- CrawfordCurrie - 02 May 2010

ItemTemplate edit

Summary CSRF uses relative location for redirect which violates HTTP specs
ReportedBy MartinVonGagern
Codebase trunk
SVN Range Foswiki-1.0.0, Thu, 08 Jan 2009, build 1878
AppliesTo Engine
Component CSRF
Priority Low
CurrentState Closed
WaitingFor
Checkins distro:f35b6a474418
TargetRelease minor
ReleasedIn 1.1.0
Topic revision: r9 - 04 Oct 2010, KennethLavrsen
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy