Changeset 174

Timestamp:

11/30/11 18:11:03 (12 years ago)

Author:

Kris Deugau

Message:

/trunk

Security review (See #30)

• convert error-message-passing via changepage to use the session to store the message so it can't be fiddled with in transit

File:

• trunk/dns.cgi (modified) (8 diffs)

trunk/dns.cgi

@@ -289 +289 @@
 
   $page->param(resultmsg => $webvar{resultmsg}) if $webvar{resultmsg};
-  $page->param(errmsg => $webvar{errmsg}) if $webvar{errmsg};
+  if ($session->param('errmsg')) {
+    $page->param(errmsg => $session->param('errmsg'));
+    $session->clear('errmsg');
+  }
 
   $page->param(curpage => $webvar{page});
-#  if ($webvar{del_failed}) {
-#    $page->param(del_failed => 1);
-#    $page->param(errmsg => $webvar{errmsg});
-#  }
 
   listdomains();
@@ -306 +305 @@
   fill_grouplist("grouplist");
 
-  if ($webvar{add_failed}) {
+  if ($session->param('add_failed')) {
+    $session->clear('add_failed');
     $page->param(add_failed => 1);
-    $page->param(errmsg => $webvar{errmsg});
+    $page->param(errmsg => $session->param('errmsg'));
+    $session->clear('errmsg');
     $page->param(domain => $webvar{domain});
   }
@@ -319 +320 @@
   # security check - does the user have permission to access this entity?
   if (!check_scope(id => $webvar{group}, type => 'group')) {
-    changepage(page => "newdomain", add_failed => 1, domain => $webvar{domain},
+    $session->param('add_failed', 1);
+##fixme:  domain a security risk for XSS?
+    changepage(page => "newdomain", domain => $webvar{domain},
         errmsg => "You do not have permission to add a domain to the requested group");
   }
@@ -330 +333 @@
   } else {
     logaction(0, $session->param("username"), $webvar{group}, "Failed adding domain $webvar{domain} ($msg)");
-    changepage(page => "newdomain", add_failed => 1, domain => $webvar{domain}, errmsg => $msg);
+    $session->param('add_failed', 1);
+##fixme:  domain a security risk for XSS?
+    changepage(page => "newdomain", domain => $webvar{domain}, errmsg => $msg);
   }
 
@@ -431 +436 @@
 
     $page->param(resultmsg => $webvar{resultmsg}) if $webvar{resultmsg};
-    $page->param(errmsg => $webvar{errmsg}) if $webvar{errmsg};
+    if ($session->param('errmsg')) {
+      $page->param(errmsg => $session->param('errmsg'));
+      $session->clear('errmsg');
+    }
 
   } # close "you can't edit default records" check
@@ -733 +741 @@
 
   $page->param(resultmsg => $webvar{resultmsg}) if $webvar{resultmsg};
-  $page->param(errmsg => $webvar{errmsg}) if $webvar{errmsg};
+  if ($session->param('errmsg')) {
+    $page->param(errmsg => $session->param('errmsg'));
+    $session->clear('errmsg');
+  }
   $page->param(curpage => $webvar{page});
 
@@ -1015 +1026 @@
   $page->param(resultmsg => $webvar{resultmsg}) if $webvar{resultmsg};
   $page->param(warnmsg => $webvar{warnmsg}) if $webvar{warnmsg};
-  $page->param(errmsg => $webvar{errmsg}) if $webvar{errmsg};
+  if ($session->param('errmsg')) {
+    $page->param(errmsg => $session->param('errmsg'));
+    $session->clear('errmsg');
+  }
   $page->param(curpage => $webvar{page});
 
@@ -1505 +1519 @@
 sub changepage {
   my %params = @_;      # think this works the way I want...
+
+  # cross-site scripting fixup.  instead of passing error messages by URL/form
+  # variable, put them in the session where the nasty user can't meddle.
+  if ($params{errmsg}) {
+    $session->param('errmsg', $params{errmsg});
+    delete $params{errmsg};
+  }
 
   # handle user check