From a293678acdd746e1d3a236656217e1b6dbca84fb Mon Sep 17 00:00:00 2001 From: JT Smith Date: Sun, 2 May 2004 16:15:33 +0000 Subject: [PATCH] took additional measures to prevent sql injection --- docs/changelog/6.x.x.txt | 7 +++++-- lib/WebGUI/Session.pm | 14 ++++---------- lib/WebGUI/Wobject.pm | 2 +- lib/WebGUI/Wobject/DataForm.pm | 4 ++-- lib/WebGUI/Wobject/EventsCalendar.pm | 2 +- lib/WebGUI/Wobject/MessageBoard.pm | 6 +++--- lib/WebGUI/Wobject/Poll.pm | 2 +- lib/WebGUI/Wobject/Product.pm | 8 ++++---- lib/WebGUI/Wobject/Survey.pm | 10 +++++----- lib/WebGUI/Wobject/USS.pm | 6 +++--- 10 files changed, 29 insertions(+), 32 deletions(-) diff --git a/docs/changelog/6.x.x.txt b/docs/changelog/6.x.x.txt index dd2ffbf63..9794f09a3 100644 --- a/docs/changelog/6.x.x.txt +++ b/docs/changelog/6.x.x.txt @@ -9,12 +9,15 @@ Manager and USS to the standard pagination variables. - A very special thanks to Len Kranendonk for the following security improvements. - - Disabled anonymous registration by default for better security. - - Set session timeout to 1 hour by default for better security. + - Disabled anonymous registration by default. + - Set session timeout to 1 hour by default. - Sessions now auto end themselves if they are used after their expires timeout and before the scheduler has cleaned them up. - Macros are now negated on user profile fields and authentication fields. + - Sessions are now validated against IP address to help prevent + session theft. + - Took additional measures to prevent SQL injection. - Bugfix [ 930425 ] Bug in EventsCalender, causing other wobjects to fail. - Bugfix [ 925586 ] HttpProxy ignores javascript in (thanks to Nicklous Roberts). diff --git a/lib/WebGUI/Session.pm b/lib/WebGUI/Session.pm index 2d46217b8..5f519e1d3 100644 --- a/lib/WebGUI/Session.pm +++ b/lib/WebGUI/Session.pm @@ -109,21 +109,15 @@ sub _setupSessionVars { tie %vars, 'Tie::CPHash'; if ($_[0] ne "") { %vars = WebGUI::SQL->quickHash("select * from userSession where sessionId='$_[0]'"); - if ($vars{expires} < _time()) { + if ($vars{expires} < _time() || $vars{lastIP} ne $session{env}{REMOTE_ADDR}) { + %vars = (); WebGUI::Session::end($_[0]); } if ($vars{sessionId} ne "") { $session{scratch} = WebGUI::SQL->buildHashRef("select name,value from userSessionScratch where sessionId=".quote($_[0])); - if (($session{setting}{proxiedClientAddress} eq "1") && ($ENV{HTTP_X_FORWARDED_FOR} ne "")) { - WebGUI::SQL->write("update userSession set lastPageView="._time().", - lastIP='$ENV{HTTP_X_FORWARDED_FOR}', - expires=".(_time()+$session{setting}{sessionTimeout}) - ." where sessionId='$_[0]'"); - } else { - WebGUI::SQL->write("update userSession set lastPageView="._time().", lastIP='$ENV{REMOTE_ADDR}', - expires=".(_time()+$session{setting}{sessionTimeout})." where sessionId='$_[0]'"); - } + WebGUI::SQL->write("update userSession set lastPageView="._time().", lastIP='$session{env}{REMOTE_ADDR}', + expires=".(_time()+$session{setting}{sessionTimeout})." where sessionId='$_[0]'"); } else { start(1,$_[0]); } diff --git a/lib/WebGUI/Wobject.pm b/lib/WebGUI/Wobject.pm index bfc5221a4..48f974934 100644 --- a/lib/WebGUI/Wobject.pm +++ b/lib/WebGUI/Wobject.pm @@ -1479,7 +1479,7 @@ sub www_paste { ."templatePosition=1, " ."sequenceNumber=". $nextSeq .", " ."bufferUserId=NULL, bufferDate=NULL, bufferPrevId=NULL " - ."WHERE wobjectId=". $session{form}{wid} ); + ."WHERE wobjectId=".$_[0]->get("wobjectId")); return ""; } else { return WebGUI::Privilege::insufficient(); diff --git a/lib/WebGUI/Wobject/DataForm.pm b/lib/WebGUI/Wobject/DataForm.pm index 1f6a8cfbe..a3f38fd6d 100644 --- a/lib/WebGUI/Wobject/DataForm.pm +++ b/lib/WebGUI/Wobject/DataForm.pm @@ -452,7 +452,7 @@ sub uiLevel { sub www_deleteEntry { return WebGUI::Privilege::insufficient() unless (WebGUI::Privilege::canEditWobject($_[0]->get("wobjectId"))); my $entryId = $session{form}{entryId}; - WebGUI::SQL->write("delete from DataForm_entry where DataForm_entryId=".$entryId); + WebGUI::SQL->write("delete from DataForm_entry where DataForm_entryId=".quote($entryId)); $session{form}{entryId} = 'list'; return $_[0]->www_view(); } @@ -880,7 +880,7 @@ sub www_process { $var->{error_loop} = \@errors; $var = $_[0]->getRecordTemplateVars($var); if ($hadErrors && !$updating) { - WebGUI::SQL->write("delete from DataForm_entryData where DataForm_entryId=".$entryId); + WebGUI::SQL->write("delete from DataForm_entryData where DataForm_entryId=".quote($entryId)); $_[0]->deleteCollateral("DataForm_entry","DataForm_entryId",$entryId); $_[0]->www_view($var); } else { diff --git a/lib/WebGUI/Wobject/EventsCalendar.pm b/lib/WebGUI/Wobject/EventsCalendar.pm index c9c3d1499..24dadfeaa 100644 --- a/lib/WebGUI/Wobject/EventsCalendar.pm +++ b/lib/WebGUI/Wobject/EventsCalendar.pm @@ -353,7 +353,7 @@ sub www_editEventSave { } else { WebGUI::SQL->write("update EventsCalendar_event set name=".quote($session{form}{name}).", description=".quote($session{form}{description}).", startDate=".$startDate[0].", - endDate=".$endDate[0]." where EventsCalendar_eventId=$session{form}{eid}"); + endDate=".$endDate[0]." where EventsCalendar_eventId=".quote($session{form}{eid})); } if ($session{form}{proceed} eq "addEvent") { $session{form}{eid} = "new"; diff --git a/lib/WebGUI/Wobject/MessageBoard.pm b/lib/WebGUI/Wobject/MessageBoard.pm index 213b7348c..087c1ff77 100644 --- a/lib/WebGUI/Wobject/MessageBoard.pm +++ b/lib/WebGUI/Wobject/MessageBoard.pm @@ -150,7 +150,7 @@ sub www_deleteForumConfirm { my $forum = WebGUI::Forum->new($session{form}{forumId}); $forum->purge; } - WebGUI::SQL->write("delete from MessageBoard_forums where forumId=".$session{form}{forumId}." and wobjectId=".$_[0]->get("wobjectId")); + WebGUI::SQL->write("delete from MessageBoard_forums where forumId=".quote($session{form}{forumId})." and wobjectId=".$_[0]->get("wobjectId")); return ""; } @@ -205,11 +205,11 @@ sub www_editForumSave { my ($seq) = WebGUI::SQL->quickArray("select max(sequenceNumber) from MessageBoard_forums where wobjectId=".$_[0]->get("wobjectId")); $seq++; WebGUI::SQL->write("insert into MessageBoard_forums (wobjectId, forumId, title, description, sequenceNumber) values (" - .$_[0]->get("wobjectId").", ".$forumId.", ".quote($session{form}{title}).", ".quote($session{form}{description}) + .$_[0]->get("wobjectId").", ".quote($forumId).", ".quote($session{form}{title}).", ".quote($session{form}{description}) .", ".$seq.")"); } else { WebGUI::SQL->write("update MessageBoard_forums set title=".quote($session{form}{title}).", description=" - .quote($session{form}{description})." where forumId=".$forumId." and wobjectId=".$_[0]->get("wobjectId")); + .quote($session{form}{description})." where forumId=".quote($forumId)." and wobjectId=".$_[0]->get("wobjectId")); } return ""; } diff --git a/lib/WebGUI/Wobject/Poll.pm b/lib/WebGUI/Wobject/Poll.pm index 9a2ba9b34..d7ff6de74 100644 --- a/lib/WebGUI/Wobject/Poll.pm +++ b/lib/WebGUI/Wobject/Poll.pm @@ -279,7 +279,7 @@ sub www_vote { my $u; if ($session{form}{answer} ne "" && WebGUI::Privilege::isInGroup($_[0]->get("voteGroup"),$session{user}{userId}) && !($_[0]->_hasVoted())) { WebGUI::SQL->write("insert into Poll_answer values (".$_[0]->get("wobjectId").", - '$session{form}{answer}', $session{user}{userId}, '$session{env}{REMOTE_ADDR}')"); + ".quote($session{form}{answer}).", $session{user}{userId}, '$session{env}{REMOTE_ADDR}')"); if ($session{setting}{useKarma}) { $u = WebGUI::User->new($session{user}{userId}); $u->karma($_[0]->get("karmaPerVote"),$_[0]->get("namespace")." (".$_[0]->get("wobjectId").")","Voted on this poll."); diff --git a/lib/WebGUI/Wobject/Product.pm b/lib/WebGUI/Wobject/Product.pm index f8abdd87c..16ee632f9 100644 --- a/lib/WebGUI/Wobject/Product.pm +++ b/lib/WebGUI/Wobject/Product.pm @@ -191,7 +191,7 @@ sub www_addAccessorySave { ($seq) = WebGUI::SQL->quickArray("select max(sequenceNumber) from Product_accessory where wobjectId=".$_[0]->get("wobjectId")); WebGUI::SQL->write("insert into Product_accessory (wobjectId,accessoryWobjectId,sequenceNumber) values - (".$_[0]->get("wobjectId").",$session{form}{accessoryWobjectId},".($seq+1).")"); + (".$_[0]->get("wobjectId").",".quote($session{form}{accessoryWobjectId}).",".($seq+1).")"); if ($session{form}{proceed}) { return $_[0]->www_addAccessory(); } else { @@ -227,7 +227,7 @@ sub www_addRelatedSave { ($seq) = WebGUI::SQL->quickArray("select max(sequenceNumber) from Product_related where wobjectId=".$_[0]->get("wobjectId")); WebGUI::SQL->write("insert into Product_related (wobjectId,relatedWobjectId,sequenceNumber) values - (".$_[0]->get("wobjectId").",$session{form}{relatedWobjectId},".($seq+1).")"); + (".$_[0]->get("wobjectId").",".quote($session{form}{relatedWobjectId}).",".($seq+1).")"); if ($session{form}{proceed}) { return $_[0]->www_addRelated(); } else { @@ -247,7 +247,7 @@ sub www_deleteAccessory { #------------------------------------------------------------------- sub www_deleteAccessoryConfirm { return WebGUI::Privilege::insufficient() unless (WebGUI::Privilege::canEditWobject($_[0]->get("wobjectId"))); - WebGUI::SQL->write("delete from Product_accessory where wobjectId=$session{form}{wid} and accessoryWobjectId=$session{form}{aid}"); + WebGUI::SQL->write("delete from Product_accessory where wobjectId=".$_[0]->get("wobjectId")." and accessoryWobjectId=".quote($session{form}{aid})); $_[0]->reorderCollateral("Product_accessory","accessoryWobjectId"); return ""; } @@ -298,7 +298,7 @@ sub www_deleteRelated { #------------------------------------------------------------------- sub www_deleteRelatedConfirm { return WebGUI::Privilege::insufficient() unless (WebGUI::Privilege::canEditWobject($_[0]->get("wobjectId"))); - WebGUI::SQL->write("delete from Product_related where wobjectId=$session{form}{wid} and relatedWobjectId=$session{form}{rid}"); + WebGUI::SQL->write("delete from Product_related where wobjectId=".$_[0]->get("wobjectId")." and relatedWobjectId=".quote($session{form}{rid})); $_[0]->reorderCollateral("Product_related","relatedWobjectId"); return ""; } diff --git a/lib/WebGUI/Wobject/Survey.pm b/lib/WebGUI/Wobject/Survey.pm index 0ea4f426a..5f58f555b 100644 --- a/lib/WebGUI/Wobject/Survey.pm +++ b/lib/WebGUI/Wobject/Survey.pm @@ -436,7 +436,7 @@ sub www_deleteAnswer { #------------------------------------------------------------------- sub www_deleteAnswerConfirm { return WebGUI::Privilege::insufficient() unless (WebGUI::Privilege::canEditWobject($_[0]->get("wobjectId"))); - WebGUI::SQL->write("delete from Survey_response where Survey_answerId=$session{form}{aid}"); + WebGUI::SQL->write("delete from Survey_response where Survey_answerId=".quote($session{form}{aid})); $_[0]->deleteCollateral("Survey_answer","Survey_answerId",$session{form}{aid}); $_[0]->reorderCollateral("Survey_answer","Survey_answerId","Survey_id"); return $_[0]->www_editQuestion; @@ -452,8 +452,8 @@ sub www_deleteQuestion { #------------------------------------------------------------------- sub www_deleteQuestionConfirm { return WebGUI::Privilege::insufficient() unless (WebGUI::Privilege::canEditWobject($_[0]->get("wobjectId"))); - WebGUI::SQL->write("delete from Survey_answer where Survey_questionId=$session{form}{qid}"); - WebGUI::SQL->write("delete from Survey_response where Survey_questionId=$session{form}{qid}"); + WebGUI::SQL->write("delete from Survey_answer where Survey_questionId=".quote($session{form}{qid})); + WebGUI::SQL->write("delete from Survey_response where Survey_questionId=".quote($session{form}{qid})); $_[0]->deleteCollateral("Survey_question","Survey_questionId",$session{form}{qid}); $_[0]->reorderCollateral("Survey_question","Survey_questionId","Survey_id"); return $_[0]->www_edit; @@ -469,8 +469,8 @@ sub www_deleteResponse { #------------------------------------------------------------------- sub www_deleteResponseConfirm { return "" unless (WebGUI::Privilege::isInGroup($_[0]->get("groupToViewReports"))); - WebGUI::SQL->write("delete from Survey_response where Survey_responseId=".$session{form}{responseId}); - WebGUI::SQL->write("delete from Survey_questionResponse where Survey_responseId=".$session{form}{responseId}); + WebGUI::SQL->write("delete from Survey_response where Survey_responseId=".quote($session{form}{responseId})); + WebGUI::SQL->write("delete from Survey_questionResponse where Survey_responseId=".quote($session{form}{responseId})); return $_[0]->www_viewGradebook; } diff --git a/lib/WebGUI/Wobject/USS.pm b/lib/WebGUI/Wobject/USS.pm index 374853be7..77443d2ea 100644 --- a/lib/WebGUI/Wobject/USS.pm +++ b/lib/WebGUI/Wobject/USS.pm @@ -238,7 +238,7 @@ sub www_approveSubmission { tie %submission, 'Tie::CPHash'; if (WebGUI::Privilege::isInGroup(4,$session{user}{userId}) || WebGUI::Privilege::isInGroup(3,$session{user}{userId})) { %submission = WebGUI::SQL->quickHash("select * from USS_submission where USS_submissionId=$session{form}{sid}"); - WebGUI::SQL->write("update USS_submission set status='Approved' where USS_submissionId=$session{form}{sid}"); + WebGUI::SQL->write("update USS_submission set status='Approved' where USS_submissionId=".quote($session{form}{sid})); WebGUI::MessageLog::addInternationalizedEntry($submission{userId},'',WebGUI::URL::page('func=viewSubmission&wid='. $session{form}{wid}.'&sid='.$session{form}{sid}),4,$_[0]->get("namespace")); WebGUI::MessageLog::completeEntry($session{form}{mlog}); @@ -297,7 +297,7 @@ sub www_denySubmission { tie %submission, 'Tie::CPHash'; if (WebGUI::Privilege::isInGroup(4,$session{user}{userId}) || WebGUI::Privilege::isInGroup(3,$session{user}{userId})) { %submission = WebGUI::SQL->quickHash("select * from USS_submission where USS_submissionId=$session{form}{sid}"); - WebGUI::SQL->write("update USS_submission set status='Denied' where USS_submissionId=$session{form}{sid}"); + WebGUI::SQL->write("update USS_submission set status='Denied' where USS_submissionId=".quote($session{form}{sid})); WebGUI::MessageLog::addInternationalizedEntry($submission{userId},'',WebGUI::URL::page('func=viewSubmission&wid='. $session{form}{wid}.'&sid='.$session{form}{sid}),5,$_[0]->get("namespace")); WebGUI::MessageLog::completeEntry($session{form}{mlog}); @@ -815,7 +815,7 @@ sub www_viewSubmission { forumId=>$submission->{forumId} }); } - WebGUI::SQL->write("update USS_submission set views=views+1 where USS_submissionId=$session{form}{sid}"); + WebGUI::SQL->write("update USS_submission set views=views+1 where USS_submissionId=".quote($session{form}{sid})); $var{title} = $submission->{title}; $var{content} = WebGUI::HTML::filter($submission->{content},$_[0]->get("filterContent")); $var{content} =~ s/\^\-\;//g;