Project

General

Profile

Bug #11105

Security issue: Redirect-Secret can be set by anyone

Added by Roel Standaert 6 months ago. Updated 5 months ago.

Status:
Closed
Priority:
High
Target version:
Start date:
11/23/2022
Due date:
% Done:

100%

Estimated time:

Description

WApplication::encodeUntrustedUrl relies on a secret in WebController (redirectSecret_). This is normally randomly generated.

However, a fix for issue #4207 introduced a regression here. We will overwrite redirectSecret_ with whatever is found in the Redirect-Secret header, allowing anyone to reassign this variable.

There's also a concurrency issue: many different sessions may concurrently write to redirectSecret_. In fact, I stumbled upon this when running a Wt build with -fsanitize=thread (when looking into issue #11101).

To address this:

  • Make assignment to redirectSecret_ atomic
  • Only look at the header if we're behind a (trusted) reverse proxy
  • Discard any such header before passing it on to the downstream session process
  • Rename the header to something like X-Wt-Redirect-Secret to indicate that it is non-standard
#1

Updated by Roel Standaert 6 months ago

The vulnerabilities are:

  • potential crashes (DoS) by repeatedly changing the Redirect-Secret token (concurrent modifications leading to segmentation faults, and interfering with external links)
  • by setting Redirect-Secret to a token that the attacker knows, an open redirect URL can be created: https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html
    • Side note: the hash is created with MD5 with a secret of 32 alphanumeric characters (with capital letters, 62 possibilities), even though it is only MD5, brute forcing it doesn't seem very feasible
#2

Updated by Roel Standaert 6 months ago

  • Status changed from New to InProgress
  • Assignee set to Roel Standaert
#3

Updated by Roel Standaert 6 months ago

  • Status changed from InProgress to Review
  • Assignee changed from Roel Standaert to Korneel Dumon
#4

Updated by Roel Standaert 6 months ago

  • Description updated (diff)
#5

Updated by Roel Standaert 5 months ago

  • % Done changed from 0 to 100
#6

Updated by Roel Standaert 5 months ago

  • Status changed from Review to Resolved
#7

Updated by Roel Standaert 5 months ago

  • Assignee changed from Korneel Dumon to Roel Standaert
#8

Updated by Roel Standaert 5 months ago

  • Target version changed from 4.9.0 to 4.8.3
#9

Updated by Roel Standaert 5 months ago

  • Status changed from Resolved to Closed
#10

Updated by Roel Standaert 5 months ago

  • Private changed from Yes to No

Also available in: Atom PDF