diff options
author | Yorhel <git@yorhel.nl> | 2020-04-20 10:31:26 +0200 |
---|---|---|
committer | Yorhel <git@yorhel.nl> | 2020-04-20 10:31:42 +0200 |
commit | b6f98d51910e0a9252b18f3b160e32d97ba64b2c (patch) | |
tree | e06fa29a3e6ffd242a56437191ec19a4a8d9cf7a /lib | |
parent | d16f10f3fc504b40bbc2ba859ca6807c77d0939d (diff) |
Strengthen ?view= query parameter security with a CSRF token
Easier than moderating the site to prevent people from creating internal
links to sensitive pages. It also turns out that SameSite cookies are
included when opening a bookmark (okayish) or clicking on a link from
outside the browser (not okay), this protects against those scenarios as
well.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/VNWeb/Images/Vote.pm | 17 | ||||
-rw-r--r-- | lib/VNWeb/Validation.pm | 19 |
2 files changed, 25 insertions, 11 deletions
diff --git a/lib/VNWeb/Images/Vote.pm b/lib/VNWeb/Images/Vote.pm index 97419901..507590c8 100644 --- a/lib/VNWeb/Images/Vote.pm +++ b/lib/VNWeb/Images/Vote.pm @@ -69,12 +69,13 @@ sub my_votes { my $SEND = form_compile any => { - images => $VNWeb::Elm::apis{ImageResult}[0], - single => { anybool => 1 }, - warn => { anybool => 1 }, - my_votes => { uint => 1 }, - pWidth => { uint => 1 }, # Set by JS - pHeight => { uint => 1 }, # ^ + images => $VNWeb::Elm::apis{ImageResult}[0], + single => { anybool => 1 }, + warn => { anybool => 1 }, + my_votes => { uint => 1 }, + pWidth => { uint => 1 }, # Set by JS + pHeight => { uint => 1 }, # ^ + nsfw_token => {}, }; # Fetch a list of images for the user to vote on. @@ -149,7 +150,7 @@ TUWF::get qr{/img/vote}, sub { enrich_token 1, $recent; framework_ title => 'Image flagging', sub { - elm_ 'ImageFlagging', $SEND, { images => [ reverse @$recent ], single => 0, warn => 1, my_votes => my_votes() }; + elm_ 'ImageFlagging', $SEND, { images => [ reverse @$recent ], single => 0, warn => 1, my_votes => my_votes(), nsfw_token => viewset(show_nsfw => 1) }; }; }; @@ -164,7 +165,7 @@ TUWF::get qr{/img/$RE{imgid}}, sub { enrich_token defined($l->[0]{my_sexual}) || auth->permDbmod(), $l; # XXX: permImgmod? framework_ title => "Image flagging for $id", sub { - elm_ 'ImageFlagging', $SEND, { images => $l, single => 1, warn => !tuwf->samesite(), my_votes => my_votes() }; + elm_ 'ImageFlagging', $SEND, { images => $l, single => 1, warn => !tuwf->samesite(), my_votes => my_votes(), nsfw_token => viewset(show_nsfw => 1) }; }; }; diff --git a/lib/VNWeb/Validation.pm b/lib/VNWeb/Validation.pm index c1b16a01..5658ba70 100644 --- a/lib/VNWeb/Validation.pm +++ b/lib/VNWeb/Validation.pm @@ -201,8 +201,10 @@ sub can_edit { } -# Returns { spoilers => 0-2, traits_sexual => 0/1 } -# Based on the view= query parameter or the user's preferences. +# Some user preferences can be overruled with a ?view= query parameter, +# viewget() can be used to fetch these parameters, viewset() to generate a +# query parameter with certain preferences overruled. +# # The query parameter has the following format: # view=1 -> spoilers=1, traits_sexual=<default> # view=2s -> spoilers=2, traits_sexual=1 @@ -213,8 +215,16 @@ sub can_edit { # s/S -> 1/0 traits_sexual # n/N -> 1/0 show_nsfw # Missing flags will use default. +# +# The parameter also contains a CSRF token to prevent direct links to pages +# with sensitive content. The token is domain-separated from the form CSRF +# tokens, but is otherwise generic for all pages and options, so if someone's +# token leaks, it's possible to generate links to any sensitive page for that +# particular user for several hours. sub viewget { - my($sp, $ts, $ns) = tuwf->reqGet('view') =~ /^([0-2])?([sS]?)([nN]?)$/; + my($view, $token) = tuwf->reqGet('view') =~ /^([^-]*)-(.+)$/; + $view = '' if !length($token) || !auth->csrfcheck($token, 'view'); + my($sp, $ts, $ns) = $view =~ /^([0-2])?([sS]?)([nN]?)$/; { spoilers => $sp // auth->pref('spoilers') || 0, traits_sexual => !$ts ? auth->pref('traits_sexual') : $ts eq 's', @@ -222,6 +232,7 @@ sub viewget { } } + # Creates a new 'view=' string with the given parameters. All other fields remain at their default. sub viewset { my %s = @_; @@ -229,6 +240,7 @@ sub viewset { $s{spoilers}//'', !defined $s{traits_sexual} ? '' : $s{traits_sexual} ? 's' : 'S', !defined $s{show_nsfw} ? '' : $s{show_nsfw} ? 'n' : 'N', + '-'.auth->csrftoken(0, 'view'); } @@ -236,6 +248,7 @@ sub viewset { # itself. We don't want people linking directly to spoilers or sexual content. # If we do get such a request, redirect to the same page without the ?view= # parameter. +# TODO: Also redirect if the token is invalid. TUWF::hook before => sub { return if tuwf->samesite || !length tuwf->reqGet('view'); my $qs = join '&', map { my $k=$_; my @l=tuwf->reqGets($k); map uri_escape($k).'='.uri_escape($_), @l } grep $_ ne 'view', tuwf->reqGets(); |