diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index 559db1593..3033bb470 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -2,6 +2,7 @@ - fixed #10606: shelf selector - fixed: Have just 1 list of protected groups. Use that everywhere. - fixed #10488: Map.gif is missing for Map wobject + - fixed #10553: WebGUI vulnerable to CSRF attacks 7.7.13 - fixed #10574: Creating Calendar Entry diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm index 4c7558b85..091f290d9 100644 --- a/lib/WebGUI/Asset.pm +++ b/lib/WebGUI/Asset.pm @@ -875,6 +875,7 @@ sub getEditForm { name=>"func", value=>"editSave" }); + $tabform->csrfToken(); my $assetId; my $class; if ($self->getId eq "new") { @@ -2707,19 +2708,20 @@ sub www_edit { =head2 www_editSave ( ) -Saves and updates history. If canEdit, returns www_manageAssets() if a new Asset is created, otherwise returns www_view(). Will return an insufficient Privilege if canEdit returns False. +Saves and updates history. If canEdit, returns www_manageAssets() if a new Asset is created, otherwise returns www_view(). Will return an insufficient Privilege if canEdit returns False, or if the submitted form does not pass the C<$session->form->validToken> check. NOTE: Don't try to override or overload this method. It won't work. What you are looking for is processPropertiesFromFormPost(). =cut sub www_editSave { - my $self = shift; + my $self = shift; + my $session = $self->session; ##If this is a new asset (www_add), the parent may be locked. We should still be able to add a new asset. - my $isNewAsset = $self->session->form->process("assetId") eq "new" ? 1 : 0; - return $self->session->privilege->locked() if (!$self->canEditIfLocked and !$isNewAsset); - return $self->session->privilege->insufficient() unless $self->canEdit; + my $isNewAsset = $session->form->process("assetId") eq "new" ? 1 : 0; + return $session->privilege->locked() if (!$self->canEditIfLocked and !$isNewAsset); + return $session->privilege->insufficient() unless $self->canEdit && $session->form->validToken; if ($self->session->config("maximumAssets")) { my ($count) = $self->session->db->quickArray("select count(*) from asset"); my $i18n = WebGUI::International->new($self->session, "Asset"); diff --git a/lib/WebGUI/AssetClipboard.pm b/lib/WebGUI/AssetClipboard.pm index 7c7c73780..44714f1a5 100644 --- a/lib/WebGUI/AssetClipboard.pm +++ b/lib/WebGUI/AssetClipboard.pm @@ -254,15 +254,24 @@ sub www_copy { =head2 www_copyList ( ) -Copies to clipboard assets in a list, then returns self calling method www_manageAssets(), if canEdit. Otherwise returns AdminConsole rendered insufficient privilege. + +Checks to see if the current user canEdit the parent containting the assets that +are being copied. If that's not true, or if the CSRF token is missing, then +return insufficient privileges. + +Copies the list of assets in the C form variable, checking each one for edit privileges. + +Returns the user to either the screen set by the C form variable, or to +the Asset Manager. =cut sub www_copyList { - my $self = shift; - return $self->session->privilege->insufficient() unless $self->canEdit; - foreach my $assetId ($self->session->form->param("assetId")) { - my $asset = WebGUI::Asset->newByDynamicClass($self->session,$assetId); + my $self = shift; + my $session = $self->session; + return $self->session->privilege->insufficient() unless $self->canEdit && $session->form->validToken; + foreach my $assetId ($session->form->param("assetId")) { + my $asset = WebGUI::Asset->newByDynamicClass($session,$assetId); if ($asset->canEdit) { my $newAsset = $asset->duplicate({skipAutoCommitWorkflows => 1}); $newAsset->update({ title=>$newAsset->getTitle.' (copy)'}); @@ -270,7 +279,7 @@ sub www_copyList { } } if ($self->session->form->process("proceed") ne "") { - my $method = "www_".$self->session->form->process("proceed"); + my $method = "www_".$session->form->process("proceed"); return $self->$method(); } return $self->www_manageAssets(); @@ -344,21 +353,30 @@ sub www_cut { =head2 www_cutList ( ) -Cuts assets in a list (removes to clipboard), then returns self calling method www_manageAssets(), if canEdit. Otherwise returns AdminConsole rendered insufficient privilege. +Checks to see if the current user canEdit the parent containting the assets that +are being cut. If that's not true, or if the CSRF token is missing, then +return insufficient privileges. + +Cuts the list of assets in the C form variable, checking each one for edit privileges +and to see if it's a system asset. + +Returns the user to either the screen set by the C form variable, or to +the Asset Manager. =cut sub www_cutList { my $self = shift; - return $self->session->privilege->insufficient() unless $self->canEdit; - foreach my $assetId ($self->session->form->param("assetId")) { - my $asset = WebGUI::Asset->newByDynamicClass($self->session,$assetId); + my $session = $self->session; + return $session->privilege->insufficient() unless $self->canEdit && $session->form->validToken; + foreach my $assetId ($session->form->param("assetId")) { + my $asset = WebGUI::Asset->newByDynamicClass($session,$assetId); if ($asset->canEdit && !$asset->get('isSystem')) { $asset->cut; } } - if ($self->session->form->process("proceed") ne "") { - my $method = "www_".$self->session->form->process("proceed"); + if ($session->form->process("proceed") ne "") { + my $method = "www_".$session->form->process("proceed"); return $self->$method(); } return $self->www_manageAssets(); @@ -368,22 +386,31 @@ sub www_cutList { =head2 www_duplicateList ( ) -Creates a bunch of duplicate assets under the same parent. +Checks to see if the current user canEdit the parent containting the assets that +are being duplicated. If that's not true, or if the CSRF token is missing, then +return insufficient privileges. + +Duplicates (copy and paste immediately) the list of assets in the C +form variable, checking each one for edit privileges. + +Returns the user to either the screen set by the C form variable, or to +the Asset Manager. =cut sub www_duplicateList { - my $self = shift; - return $self->session->privilege->insufficient() unless $self->canEdit; - foreach my $assetId ($self->session->form->param("assetId")) { - my $asset = WebGUI::Asset->newByDynamicClass($self->session,$assetId); + my $self = shift; + my $session = $self->session; + return $session->privilege->insufficient() unless $self->canEdit && $session->form->validToken; + foreach my $assetId ($session->form->param("assetId")) { + my $asset = WebGUI::Asset->newByDynamicClass($session,$assetId); if ($asset->canEdit) { my $newAsset = $asset->duplicate; $newAsset->update({ title=>$newAsset->getTitle.' (copy)'}); } } - if ($self->session->form->process("proceed") ne "") { - my $method = "www_".$self->session->form->process("proceed"); + if ($session->form->process("proceed") ne "") { + my $method = "www_".$session->form->process("proceed"); return $self->$method(); } return $self->www_manageAssets(); @@ -506,15 +533,21 @@ sub www_paste { =head2 www_pasteList ( ) -Pastes a selection of assets. If canEdit is False, returns an insufficient privileges page. -Returns the user to the manageAssets screen. +Checks to see if the current user canEdit the parent containting the assets that +are being pasted. If that's not true, or if the CSRF token is missing, then +return insufficient privileges. + +Pastes the list of assets in the C form variable, checking each one for edit privileges. + +Returns the user to either the screen set by the C form variable, or to +the Asset Manager. =cut sub www_pasteList { my $self = shift; my $session = $self->session; - return $session->privilege->insufficient() unless $self->canEdit; + return $session->privilege->insufficient() unless $self->canEdit && $session->form->validToken; my $form = $session->form; my $pb = WebGUI::ProgressBar->new($session); ##Need to store the list of assetIds for the status subroutine diff --git a/lib/WebGUI/AssetLineage.pm b/lib/WebGUI/AssetLineage.pm index 11eda8bc4..93b00985d 100644 --- a/lib/WebGUI/AssetLineage.pm +++ b/lib/WebGUI/AssetLineage.pm @@ -1020,8 +1020,10 @@ sub www_setRank { =head2 www_setRanks ( ) Utility method for the AssetManager. Reorders 1 pagefull of assets via rank. +AssetIds are passed in via the C form variable. -If the current user cannot edit the current asset, it returns the insufficient privileges page. +If the current user cannot edit the current asset, or if a valid CSRF token +is not submitted with the form, it returns the insufficient privileges page. Returns the user to the manage assets screen. @@ -1030,7 +1032,7 @@ Returns the user to the manage assets screen. sub www_setRanks { my $self = shift; my $session = $self->session; - return $session->privilege->insufficient() unless $session->asset->canEdit; + return $session->privilege->insufficient() unless $session->asset->canEdit && $session->form->validToken; my $i18n = WebGUI::International->new($session, 'Asset'); my $pb = WebGUI::ProgressBar->new($session); my $form = $session->form; diff --git a/lib/WebGUI/AssetTrash.pm b/lib/WebGUI/AssetTrash.pm index 2b0570aab..f02570e38 100644 --- a/lib/WebGUI/AssetTrash.pm +++ b/lib/WebGUI/AssetTrash.pm @@ -321,7 +321,11 @@ sub www_delete { =head2 www_deleteList -Moves list of assets to trash, returns www_manageAssets() method of self if canEdit. Otherwise returns AdminConsole rendered insufficient privilege. +Checks to see if a valid CSRF token was received. If not, then it returns insufficient privilege. + +Moves list of assets to trash, checking each to see if the user canEdit, +and canEditIfLocked. Returns the user to manageTrash, or to the screen set +by the form variable C. =cut @@ -333,6 +337,7 @@ sub www_deleteList { my $form = $session->form; my @assetIds = $form->param('assetId'); $pb->start($i18n->get('Delete Assets'), $session->url->extras('adminConsole/assets.gif')); + return $self->session->privilege->insufficient() unless $session->form->validToken; ASSETID: foreach my $assetId (@assetIds) { my $asset = eval { WebGUI::Asset->newPending($session,$assetId); }; if ($@) { @@ -404,6 +409,7 @@ sub www_manageTrash { $output .= ' assetManager.AddButton("'.$i18n->get("restore").'","restoreList","manageTrash"); assetManager.AddButton("'.$i18n->get("purge").'","purgeList","manageTrash"); + assetManager.AddFormHidden({ name:"webguiCsrfToken", value:"'.$self->session->scratch->get('webguiCsrfToken').'"}); assetManager.Write(); var assetListSelectAllToggle = false; function toggleAssetListSelectAll(form) { @@ -427,11 +433,14 @@ sub www_manageTrash { Purges a piece of content, including all it's revisions, from the system permanently. +Returns insufficient privileges unless the submitted form passes the validToken check. + =cut sub www_purgeList { my $self = shift; my $session = $self->session; + return $session->privilege->insufficient() unless $session->form->validToken; my $pb = WebGUI::ProgressBar->new($session); my $i18n = WebGUI::International->new($session, 'Asset'); $pb->start($i18n->get('purge'), $session->url->extras('adminConsole/assets.gif')); diff --git a/lib/WebGUI/Content/AssetManager.pm b/lib/WebGUI/Content/AssetManager.pm index 2431d5e9e..8f61193c7 100644 --- a/lib/WebGUI/Content/AssetManager.pm +++ b/lib/WebGUI/Content/AssetManager.pm @@ -397,9 +397,10 @@ ENDHTML $output .= ''; ### The page of assets - $output .= sprintf <asset->getUrl, $i18n->get( 'with selected' ), $i18n->get( "update" ), $i18n->get( "delete" ), $i18n->get( '43' ), $i18n->get( 'cut' ), $i18n->get( "Copy" ), $i18n->get( "duplicate" ); + $output .= sprintf <asset->getUrl, WebGUI::Form::CsrfToken->new($session)->toHtml, $i18n->get( 'with selected' ), $i18n->get( "update" ), $i18n->get( "delete" ), $i18n->get( '43' ), $i18n->get( 'cut' ), $i18n->get( "Copy" ), $i18n->get( "duplicate" );
+%s
diff --git a/lib/WebGUI/Form/CsrfToken.pm b/lib/WebGUI/Form/CsrfToken.pm new file mode 100644 index 000000000..51b0621e2 --- /dev/null +++ b/lib/WebGUI/Form/CsrfToken.pm @@ -0,0 +1,70 @@ +package WebGUI::Form::CsrfToken; + +=head1 LEGAL + + ------------------------------------------------------------------- + WebGUI is Copyright 2001-2009 Plain Black Corporation. + ------------------------------------------------------------------- + Please read the legal notices (docs/legal.txt) and the license + (docs/license.txt) that came with this distribution before using + this software. + ------------------------------------------------------------------- + http://www.plainblack.com info@plainblack.com + ------------------------------------------------------------------- + +=cut + +use strict; +use base 'WebGUI::Form::Hidden'; +use WebGUI::International; + +=head1 NAME + +Package WebGUI::Form::CsrfToken + +=head1 DESCRIPTION + +Creates a hidden field to use for CSRF prevention.. + +=head1 SEE ALSO + +This is a subclass of WebGUI::Form::Hidden. + +=head1 METHODS + +The following methods are specifically available from this class. Check the superclass for additional methods. + +=cut + + +#------------------------------------------------------------------- + +=head2 getName ( session ) + +Returns the human readable name of this control. + +=cut + +sub getName { + my ($self, $session) = @_; + return WebGUI::International->new($session, 'WebGUI')->get('csrfToken'); +} + +#------------------------------------------------------------------- + +=head2 toHtmlAsHidden ( ) + +Renders an input tag of type hidden. + +=cut + +sub toHtmlAsHidden { + my $self = shift; + $self->set('name', 'webguiCsrfToken'); + $self->set('value', $self->session->scratch->get('webguiCsrfToken')); + return $self->SUPER::toHtmlAsHidden(); +} + + +1; + diff --git a/lib/WebGUI/Operation/Group.pm b/lib/WebGUI/Operation/Group.pm index ef97e325a..dfd21911f 100644 --- a/lib/WebGUI/Operation/Group.pm +++ b/lib/WebGUI/Operation/Group.pm @@ -290,7 +290,7 @@ A WebGUI::Session object sub www_addGroupsToGroupSave { my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); + return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid")) && $session->form->validToken); my $group = WebGUI::Group->new($session,$session->form->process("gid")); my @groups = $session->form->group('groups'); $group->addGroups(\@groups); @@ -314,7 +314,7 @@ A WebGUI::Session object sub www_addUsersToGroupSave { my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); + return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid")) && $session->form->validToken); my @users = $session->form->selectList('users'); my $group = WebGUI::Group->new($session,$session->form->process("gid")); $group->addUsers(\@users); @@ -419,10 +419,9 @@ sub www_deleteGroupGrouping { =head2 www_deleteGrouping ( ) -Deletes a set of users from a set of groups. -The user and group lists are expected to -be found in form fields names uid and gid, respectively. Visitors are not allowed to -perform this operation. +Deletes a set of users from a set of groups. The user and group lists are expected +to be found in form fields names uid and gid, respectively. Visitors are not +allowed to perform this operation. =head3 $session @@ -432,7 +431,7 @@ A WebGUI::Session object sub www_deleteGrouping { my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); + return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid")) && $session->form->validToken); if (($session->user->userId eq $session->form->process("uid") || $session->form->process("uid") eq '3') && $session->form->process("gid") eq '3') { return $session->privilege->vitalComponent(); } @@ -476,6 +475,7 @@ sub www_editGroup { -name => "op", -value => "editGroupSave", ); + $f->csrfToken(); $f->hidden( -name => "gid", -value => $session->form->process("gid") @@ -657,7 +657,7 @@ sub www_editGroupSave { my $session = shift; my $gid = $session->form->process("gid"); return $session->privilege->adminOnly - unless canEditGroup($session, $gid); + unless canEditGroup($session, $gid) && $session->form->validToken; my $g = WebGUI::Group->new($session, $gid); # We don't want them to use an existing name. If needed, we'll add a number to the name to keep it unique. my $groupName = $session->form->process("groupName"); @@ -718,6 +718,7 @@ sub www_editGrouping { my $i18n = WebGUI::International->new($session); my $f = WebGUI::HTMLForm->new($session); $f->submit; + $f->csrfToken(); $f->hidden( -name => "op", -value => "editGroupingSave" @@ -774,7 +775,7 @@ A WebGUI::Session object sub www_editGroupingSave { my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); + return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid")) && $session->form->validToken); my $group = WebGUI::Group->new($session,$session->form->process("gid")); $group->userGroupExpireDate($session->form->process("uid"),$session->datetime->setToEpoch($session->form->process("expireDate"))); $group->userIsAdmin($session->form->process("uid"),$session->form->process("groupAdmin")); @@ -805,6 +806,7 @@ sub www_emailGroup { -name => "op", -value => "emailGroupSend" ); + $f->csrfToken(); $f->hidden( -name => "gid", -value => $session->form->process("gid") @@ -853,7 +855,7 @@ A WebGUI::Session object sub www_emailGroupSend { my $session = shift; - return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); + return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid")) && $session->form->validToken); my $mail = WebGUI::Mail::Send->create($session, {toGroup=>$session->form->process("gid"),subject=>$session->form->process("subject"),from=>$session->form->process("from")}); $mail->addHtml($session->form->process("message","HTMLArea")); $mail->addFooter; @@ -958,6 +960,7 @@ sub www_manageGroupsInGroup { return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); my $f = WebGUI::HTMLForm->new($session); + $f->csrfToken(); $f->submit; $f->hidden( -name => "op", @@ -1014,6 +1017,7 @@ sub www_manageUsersInGroup { return $session->privilege->adminOnly() unless (canEditGroup($session,$session->form->process("gid"))); my $i18n = WebGUI::International->new($session); my $output = WebGUI::Form::formHeader($session,) + .WebGUI::Form::csrfToken($session,{}) .WebGUI::Form::hidden($session,{ name=>"gid", value=>$session->form->process("gid") @@ -1036,7 +1040,6 @@ sub www_manageUsersInGroup { name=>"uid", value=>$row->{userId} }) - .$session->icon->delete('op=deleteGrouping;uid='.$row->{userId}.';gid='.$session->form->process("gid")) .$session->icon->edit('op=editGrouping;uid='.$row->{userId}.';gid='.$session->form->process("gid")) .''; $output .= ''.$row->{username}.''; @@ -1050,6 +1053,7 @@ sub www_manageUsersInGroup { return _submenu($session,$output) unless ($session->form->process("doit") || $userCount < 250 || $session->form->process("pn") > 1); my $f = WebGUI::HTMLForm->new($session); $f->submit; + $f->csrfToken(); $f->hidden( -name => "gid", -value => $session->form->process("gid") diff --git a/lib/WebGUI/Operation/Settings.pm b/lib/WebGUI/Operation/Settings.pm index acbd9fe5e..96cb827f0 100644 --- a/lib/WebGUI/Operation/Settings.pm +++ b/lib/WebGUI/Operation/Settings.pm @@ -607,6 +607,7 @@ sub www_editSettings { name => "op", value => "saveSettings" }); + $tabform->csrfToken(); my $definitions = definition($session, $i18n); foreach my $definition (@{$definitions}) { @@ -671,7 +672,7 @@ is in group Admin (3). Returns the user to the Edit Settings screen, www_editSe sub www_saveSettings { my $session = shift; - return $session->privilege->adminOnly() unless ($session->user->isAdmin); + return $session->privilege->adminOnly() unless ($session->user->isAdmin && $session->form->validToken); my $i18n = WebGUI::International->new($session, "WebGUI"); my $setting = $session->setting; my $form = $session->form; diff --git a/lib/WebGUI/Operation/VersionTag.pm b/lib/WebGUI/Operation/VersionTag.pm index 1b8e7e205..c9c604989 100644 --- a/lib/WebGUI/Operation/VersionTag.pm +++ b/lib/WebGUI/Operation/VersionTag.pm @@ -154,7 +154,7 @@ sub www_approveVersionTag { my $tag = WebGUI::VersionTag->new( $session, $session->form->param("tagId") ); return $session->privilege->insufficient - unless canApproveVersionTag( $session, $tag ); + unless canApproveVersionTag( $session, $tag ) && $session->form->validToken; my $instance = $tag->getWorkflowInstance; my $activity = $instance->getNextActivity; @@ -217,6 +217,7 @@ sub www_editVersionTag { -value=>"editVersionTagSave" ); my $value = $tag->getId if defined $tag; + $f->csrfToken(); $f->hidden( -name=>"tagId", -value=>$value, @@ -271,7 +272,7 @@ A reference to the current session. sub www_editVersionTagSave { my $session = shift; - return $session->session->privilege->insufficient() unless canView($session); + return $session->session->privilege->insufficient() unless canView($session) && $session->form->validToken; if ($session->form->param("tagId") eq "new") { my $tag = WebGUI::VersionTag->create($session, { name=>$session->form->process("name","text", "Untitled"), @@ -323,6 +324,7 @@ sub www_commitVersionTag { # Commit comments form my $f = WebGUI::HTMLForm->new($session); $f->submit; + $f->csrfToken(); $f->readOnly( label => $i18n->get("version tag name"), hoverHelp => $i18n->get("version tag name description commit"), @@ -419,7 +421,7 @@ sub www_commitVersionTagConfirm { my $tagId = $session->form->param("tagId"); if ($tagId) { my $tag = WebGUI::VersionTag->new($session, $tagId); - if (defined $tag && $session->user->isInGroup($tag->get("groupToUse"))) { + if (defined $tag && $session->user->isInGroup($tag->get("groupToUse")) && $session->form->validToken) { my $i18n = WebGUI::International->new($session, "VersionTag"); my $startTime = WebGUI::DateTime->new($session,$session->form->process("startTime","dateTime"))->toDatabase; @@ -653,8 +655,9 @@ sub www_manageRevisionsInTag { $ac->addSubmenuItem($session->url->page('op=manageVersions'), $i18n->get("manage versions")); # Process any actions - my $action = lc $session->form->get('action'); - if ( $action eq "purge" ) { + my $action = lc $session->form->get('action'); + my $validToken = $session->form->validToken; + if ( $action eq "purge" && $validToken) { # Purge these revisions my @assetInfo = $session->form->get('assetInfo'); for my $assetInfo ( @assetInfo ) { @@ -669,7 +672,7 @@ sub www_manageRevisionsInTag { return www_manageVersions( $session ); } } - elsif ( $action eq "move to:" ) { + elsif ( $action eq "move to:" && $validToken) { # Get the new version tag my $moveToTagId = $session->form->get('moveToTagId'); my $moveToTag; @@ -697,7 +700,7 @@ sub www_manageRevisionsInTag { return www_manageVersions( $session ); } } - elsif ( $action eq "update version tag" ) { + elsif ( $action eq "update version tag" && $validToken) { my $startTime = WebGUI::DateTime->new($session,$session->form->process("startTime","dateTime"))->toDatabase; my $endTime = WebGUI::DateTime->new($session,$session->form->process("endTime","dateTime"))->toDatabase; @@ -716,6 +719,7 @@ sub www_manageRevisionsInTag { if (defined $instance) { my $form = WebGUI::HTMLForm->new($session); $form->submit; + $form->csrfToken; $form->hidden( name=>"tagId", value=>$tagId @@ -769,6 +773,7 @@ sub www_manageRevisionsInTag { .= WebGUI::Form::formHeader( $session, {} ) . WebGUI::Form::hidden( $session, { name => 'op', value=> 'manageRevisionsInTag' } ) . WebGUI::Form::hidden( $session, { name => 'tagId', value => $tag->getId } ) + . WebGUI::Form::csrfToken( $session ) . '' . '' . '
' diff --git a/lib/WebGUI/Session/Form.pm b/lib/WebGUI/Session/Form.pm index db3b865a3..218d103f7 100644 --- a/lib/WebGUI/Session/Form.pm +++ b/lib/WebGUI/Session/Form.pm @@ -174,5 +174,24 @@ sub process { }); } +#------------------------------------------------------------------- + +=head2 validToken ( ) + +Checks that the current form has a method=POST, and that it has a CSRF token matching +the one in this user's current session. + +=cut + +sub validToken { + my ($self) = @_; + my $session = $self->session; + $session->log->warn('method: '. $session->request->method); + $session->log->warn('token: '. $session->scratch->get('webguiCsrfToken')); + return 0 unless $session->request->method eq 'POST'; + return 0 unless $self->param('webguiCsrfToken') eq $session->scratch->get('webguiCsrfToken'); + return 1; +} + 1; diff --git a/lib/WebGUI/Session/Var.pm b/lib/WebGUI/Session/Var.pm index 3688cce3b..d472c0cab 100644 --- a/lib/WebGUI/Session/Var.pm +++ b/lib/WebGUI/Session/Var.pm @@ -194,6 +194,7 @@ sub new { $self->{_var}{expires} = $session->datetime->time() + $session->setting->get("sessionTimeout"); $self->session->{_sessionId} = $self->{_var}{sessionId}; $session->db->setRow("userSession","sessionId",$self->{_var}); + return $self; } else { ##Start a new default session with the requested, non-existant id. $self->start(1,$sessionId); @@ -222,7 +223,7 @@ sub session { =head2 start ( [ userId, sessionId ] ) Start a new user session. Returns the user session id. The session variable's sessionId -is set to the var object's session id. +is set to the var object's session id. Also sets the user's CSRF token. =head3 userId @@ -251,6 +252,7 @@ sub start { $self->{_var}{sessionId} = $sessionId; $self->session->db->setRow("userSession","sessionId",$self->{_var},$sessionId); $self->session->{_sessionId} = $sessionId; + $self->session->scratch->set('webguiCsrfToken', $self->session->id->generate); } #------------------------------------------------------------------- diff --git a/lib/WebGUI/TabForm.pm b/lib/WebGUI/TabForm.pm index daa1676f3..7d92359dc 100644 --- a/lib/WebGUI/TabForm.pm +++ b/lib/WebGUI/TabForm.pm @@ -102,6 +102,20 @@ sub addTab { return $self->{_tab}{$name}{form}; } +#------------------------------------------------------------------- + +=head2 csrfToken ( ) + +Adds the WebGUI CSRF token to the form. Really a wrapper for WebGUI::Form::CsrfToken. + +=cut + +sub csrfToken { + my $self = shift; + $self->{_hidden} .= WebGUI::Form::CsrfToken($self->session); +} + + #------------------------------------------------------------------- =head2 formHeader ( hashRef ) diff --git a/lib/WebGUI/i18n/English/WebGUI.pm b/lib/WebGUI/i18n/English/WebGUI.pm index 9e68cde90..7efc2df0f 100644 --- a/lib/WebGUI/i18n/English/WebGUI.pm +++ b/lib/WebGUI/i18n/English/WebGUI.pm @@ -4475,6 +4475,12 @@ Users may override this setting in their profile. lastUpdated => 0, }, + 'csrfToken' => { + message => 'CSRF Token', + lastUpdated => 0, + context => 'CSRF = Cross Site Request Forgery, token is a piece of identification', + }, + }; 1; diff --git a/t/Form/CsrfToken.t b/t/Form/CsrfToken.t new file mode 100644 index 000000000..d8972091e --- /dev/null +++ b/t/Form/CsrfToken.t @@ -0,0 +1,58 @@ +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2009 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------- +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------- + +use FindBin; +use strict; +use lib "$FindBin::Bin/../lib"; + +use WebGUI::Test; +use WebGUI::Form; +use WebGUI::Form::CsrfToken; +use WebGUI::Session; +use HTML::Form; +use WebGUI::Form_Checking; + +#The goal of this test is to verify that Zipcode form elements work + +use Test::More; # increment this value for each test you create + +my $session = WebGUI::Test->session; + +# put your tests here + +my $formClass = 'WebGUI::Form::CsrfToken'; + +my $numTests = 5; + +plan tests => $numTests; + +my ($header, $footer) = (WebGUI::Form::formHeader($session), WebGUI::Form::formFooter($session)); + +my $html = join "\n", + $header, + $formClass->new($session, {})->toHtml, + $footer; + +my @forms = HTML::Form->parse($html, 'http://www.webgui.org'); + +##Test Form Generation + +is(scalar @forms, 1, '1 form was parsed'); + +my @inputs = $forms[0]->inputs; +is(scalar @inputs, 1, 'The form has 1 input'); + +#Basic tests + +my $input = $inputs[0]; +is($input->name, 'webguiCsrfToken', 'Checking input name'); +is($input->type, 'hidden', 'Checking input type'); +is($input->value, $session->scratch->get('webguiCsrfToken'), 'Checking token value'); + diff --git a/t/Session/Form.t b/t/Session/Form.t new file mode 100644 index 000000000..c094b06ad --- /dev/null +++ b/t/Session/Form.t @@ -0,0 +1,37 @@ +#------------------------------------------------------------------- +# WebGUI is Copyright 2001-2009 Plain Black Corporation. +#------------------------------------------------------------------- +# Please read the legal notices (docs/legal.txt) and the license +# (docs/license.txt) that came with this distribution before using +# this software. +#------------------------------------------------------------------- +# http://www.plainblack.com info@plainblack.com +#------------------------------------------------------------------- + +use FindBin; +use strict; +use lib "$FindBin::Bin/../lib"; + +use WebGUI::Test; +use WebGUI::Session; + +use Test::More; + +plan tests => 4; + +my $session = WebGUI::Test->session; +my $token = $session->scratch->get('webguiCsrfToken'); + +$session->request->method('POST'); +$session->request->setup_param({ webguiCsrfToken => $token, }); +ok($session->form->validToken, 'validToken: right method and form value'); + +$session->request->method('GET'); +ok(! $session->form->validToken, '... wrong method, right form value'); + +$session->request->method('POST'); +$session->request->setup_param({ webguiCsrfToken => 'bad token', }); +ok(! $session->form->validToken, 'validToken: right method and wrong form value'); + +$session->request->method('GET'); +ok(! $session->form->validToken, 'validToken: wrong method and form value'); diff --git a/t/Session/Var.t b/t/Session/Var.t index bc78f4dc9..4667ed1ff 100644 --- a/t/Session/Var.t +++ b/t/Session/Var.t @@ -16,7 +16,7 @@ use WebGUI::Test; use WebGUI::Session; use WebGUI::Session::Var; -use Test::More tests => 40; # increment this value for each test you create +use Test::More tests => 44; # increment this value for each test you create use Test::Deep; my $session = WebGUI::Test->session; @@ -29,9 +29,13 @@ is($session->var->isAdminOn, 1, "switchAdminOn()"); $session->var->switchAdminOff; is($session->var->isAdminOn, 0, "switchAdminOff()"); +my $token = $session->scratch->get('webguiCsrfToken'); +ok( $token, 'CSRF token set'); +ok( $session->id->valid($token), '...is a valid GUID'); + my $id = $session->var->getId; my ($count) = $session->db->quickArray("select count(*) from userSession where sessionId=?",[$id]); -is($count, 1,"created an user session entry in the database"); +is($count, 1, "created an user session entry in the database"); my %newEnvHash = ( REMOTE_ADDR => '192.168.0.34'); my $origEnv = $session->env->{_env}; @@ -41,6 +45,8 @@ my $var = WebGUI::Session::Var->new($session); my $varTime = time(); my $varExpires = $varTime + $session->setting->get('sessionTimeout'); isa_ok($var, 'WebGUI::Session::Var', 'new returns Var object'); +isnt($session->scratch->get('webguiCsrfToken'), $token, '... calling new without sessionId creates a new token'); +$token = $session->scratch->get('webguiCsrfToken'); cmp_ok(abs($var->get('lastPageView') - $varTime), '<=', 1, 'lastPageView set correctly'); cmp_ok(abs($var->get('expires') - $varExpires), '<=', 1, 'expires set correctly'); @@ -66,6 +72,7 @@ $newEnvHash{REMOTE_ADDR} = '10.0.5.5'; $varTime = time(); my $var2 = WebGUI::Session::Var->new($session, $session->getId); $varExpires = $varTime + $session->setting->get('sessionTimeout'); +is($var2->session->scratch->get('webguiCsrfToken'), $token, 'opening a new user session did not change the CSRF token'); cmp_deeply( $var2, diff --git a/t/lib/WebGUI/PseudoRequest.pm b/t/lib/WebGUI/PseudoRequest.pm index 18f2de8a4..8b98e228b 100644 --- a/t/lib/WebGUI/PseudoRequest.pm +++ b/t/lib/WebGUI/PseudoRequest.pm @@ -378,6 +378,23 @@ sub get_output { #---------------------------------------------------------------------------- +=head2 method ( [ $method ] ) + +Getter/setter for the HTTP request method. + +=cut + +sub method { + my ($self, $newMethod) = @_; + my $method = $self->{method}; + if (defined $newMethod) { + $self->{method} = $newMethod; + } + return $method; +} + +#---------------------------------------------------------------------------- + =head2 print ( @values ) Fake print method for the PseudoRequest object. It caches everything printed diff --git a/www/extras/assetManager/assetManager.js b/www/extras/assetManager/assetManager.js index c7f8c913a..31d50976a 100644 --- a/www/extras/assetManager/assetManager.js +++ b/www/extras/assetManager/assetManager.js @@ -16,16 +16,18 @@ function AssetManager() { this.dosort=true; this.tablecontainsforms=false; // Methods - this.AddLine = AssetManager_AddLine; - this.AddColumn = AssetManager_AddColumn; - this.AddButton = AssetManager_AddButton; - this.Write = AssetManager_Write; - this.SortRows = AssetManager_SortRows; + this.AddLine = AssetManager_AddLine; + this.AddColumn = AssetManager_AddColumn; + this.AddButton = AssetManager_AddButton; + this.Write = AssetManager_Write; + this.SortRows = AssetManager_SortRows; this.AddLineSortData = AssetManager_AddLineSortData; + this.AddFormHidden = AssetManager_AddFormHidden; // Structure - this.Columns = new Array(); - this.Lines = new Array(); - this.Buttons = new Array(); + this.Columns = new Array(); + this.Lines = new Array(); + this.Buttons = new Array(); + this.FormHidden = new Array(); //***************Properties used for dragging @@ -62,6 +64,12 @@ function AssetManager() { } +// Add a hidden input field to the form +function AssetManager_AddFormHidden( hiddenObj ) { + var index = this.FormHidden.length; + this.FormHidden[index] = hiddenObj; +} + // Add a line to the grid function AssetManager_AddLine() { var index = this.Lines.length; @@ -71,7 +79,7 @@ function AssetManager_AddLine() { this.Lines[index][i].text = arguments[i]; this.Lines[index][i].data = arguments[i]; } - } +} // Add a button to the form function AssetManager_AddButton(label,func,proceed) { @@ -161,6 +169,10 @@ function AssetManager_Write() { for (var j=0; j'); } + for (var j=0; j < this.FormHidden.length; j++) { + var myHidden = this.FormHidden[j]; + document.write(''); + } document.write(''); }