Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional changes for #492 #493

Merged
merged 3 commits into from
Dec 7, 2018
Merged

Additional changes for #492 #493

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c0e810
Add token to prevent CSRF.
mpkut Nov 29, 2018
d24508d
Revert changes on tempates.
ikedas Dec 5, 2018
b30815c
Decorate output to insert CSRF token.
ikedas Dec 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 76 additions & 4 deletions src/cgi/wwsympa.fcgi.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,23 @@ our %required_privileges = (
#XXX'automatic_lists_management' => ['listmaster'],
);

# An action is a candidate for this list if it modifies an object or setting.
#
# Why not just protect all actions? Many of them are used in GET requests
# without any forms, making it more difficult to supply a CSRF token.
# This list intentionally starts out small in the name of breaking as little
# as possible.

our %require_csrftoken = (
'add' => 1,
'confirm_action' => 1,
'del' => 1,
'move_user' => 1,
'savefile' => 1,
'setpasswd' => 1,
'setpref' => 1,
);

# this definition is used to choose the left side menu type (admin ->
# listowner admin menu | serveradmin -> server_admin menu | none list or
# your_list menu)
Expand Down Expand Up @@ -979,6 +996,9 @@ our %in_regexp = (

# Role
'role' => 'member|editor|owner',

## CSRF token is a lower case MD5 hash
'csrftoken' => '^[0-9a-f]{32}$',
);

## Regexp applied on incoming parameters (%in)
Expand Down Expand Up @@ -1260,11 +1280,19 @@ while ($query = CGI::Fast->new) {
$in{'oauth_token'} = delete $session->{'oauth_token'}
if $oauth_token ne '' && $session->{'email'} ne 'nobody';

# Generate session-specific CSRF token
if (not defined($session->{'csrftoken'})) {
$session->{'csrftoken'} =
Digest::MD5::md5_hex(sprintf("%d %d", time, rand 0xFFFFFFFF));
wwslog('debug', "Session CSRF token: %s", $session->{'csrftoken'});
}

$param->{'session'} = $session->as_hashref();

$log->{level} = $session->{'log_level'} if ($session->{'log_level'});
$param->{'restore_email'} = $session->{'restore_email'};
$param->{'dumpvars'} = $session->{'dumpvars'};
$param->{'csrftoken'} = $session->{'csrftoken'};

## RSS does not require user authentication
unless ($rss) {
Expand Down Expand Up @@ -2286,6 +2314,30 @@ sub check_action_parameters {
}
}

## Validate CSRF token when one is required
if (defined($require_csrftoken{$param->{'action'}})) {
wwslog('debug', 'Action %s: CSRF token required', $param->{'action'});

unless (defined($in{'csrftoken'})
and ($in{'csrftoken'} eq $session->{'csrftoken'})) {
Sympa::WWW::Report::reject_report_web('user',
'authorization_reject', {'list' => $in{'list'}},
$param->{'action'}, '');

wwslog('info', 'CSRF token mismatch: in="%s" session="%s"',
$in{'csrftoken'}, $session->{'csrftoken'});
web_db_log(
{ 'status' => 'error',
'error_type' => 'authorization'
}
);
delete $param->{'list'};
# invalidate the CSRF token so a new one will be generated
delete $session->{'csrftoken'};
return undef;
}
}

## Check required privileges
if (defined $required_privileges{$action}) {
## There may be alternate privileges
Expand Down Expand Up @@ -2431,8 +2483,9 @@ sub send_html {
@other_include_path = ();

# Then output the content.
my $output = '';
unless (
$template->parse($param_copy, $tt2_file, \*STDOUT, has_header => 1)) {
$template->parse($param_copy, $tt2_file, \$output, has_header => 1)) {
my $error = $template->{last_error};

if ( $param->{'action'} eq 'help'
Expand All @@ -2453,10 +2506,29 @@ sub send_html {
my $error_escaped = Sympa::Tools::Text::encode_html($error);
$param->{'tt2_error'} = $error_escaped;
$param_copy->{'tt2_error'} = $error_escaped;
$template->parse($param_copy, 'tt2_error.tt2', \*STDOUT,
$output = '';
$template->parse($param_copy, 'tt2_error.tt2', \$output,
has_header => 1);
print STDOUT "\n\n"; # when tt2 failed to parse
}
$output .= "\n\n"; # when tt2 failed to parse
}

# Insert CSRF token.
if ($session->{'csrftoken'}) {
my $csrf_field =
sprintf '<input type="hidden" name="csrftoken" value="%s" />',
$session->{'csrftoken'};
$output =~ s{
( <form (?=\s) [^>]* \s method="post" (?=[\s>]) [^>]* > )
( .*? )
( </form> )
}{
my ($beg, $content, $end) = ($1, $2, $3);
$content =~ s/( <fieldset (?=[\s>]) [^>]* > )/$1$csrf_field/ix
or $content =~ s/\A/$csrf_field/;
$beg . $content . $end;
}egisx;
}
print $output;
}

sub prepare_report_user {
Expand Down