CVE-2015-0917: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in the backend in Kajona before 4.6.3 allows remote attackers to inject arbitrary web script or HTML via the action parameter to index.php.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
Published: 2015-01-08T15:59Z
Repository: https://github.com/kajona/kajonacms
Language: PHP
Files changed: module_system/admin/class_admin_controller.php, module_system/system/class_abstract_controller.php
Commit date: 2015-01-05 09:29:16 +0100
Commit message: BUG: / SECURITY: Fixed a possible xss injection when handling non-existent action-names. fixes SROEADV-2015-01.

Diff: diff --git a/module_system/admin/class_admin_controller.php b/module_system/admin/class_admin_controller.php
index c0b12331b..5a6df6a5e 100644
--- a/module_system/admin/class_admin_controller.php
+++ b/module_system/admin/class_admin_controller.php
@@ -365,17 +365,16 @@ abstract class class_admin_controller extends class_abstract_controller {
      * @return string
      * @since 3.4
      */
     public function action($strAction = "") {
 
-        if($strAction == "") {
-            $strAction = $this->getAction();
-        }
-        else {
+        if($strAction != "") {
             $this->setAction($strAction);
         }
 
+        $strAction = $this->getAction();
+
         //search for the matching method - build method name
         $strMethodName = "action" . uniStrtoupper($strAction[0]) . uniSubstr($strAction, 1);
 
         if(method_exists($this, $strMethodName)) {
 
diff --git a/module_system/system/class_abstract_controller.php b/module_system/system/class_abstract_controller.php
index 375003cdf..f56efc37e 100644
--- a/module_system/system/class_abstract_controller.php
+++ b/module_system/system/class_abstract_controller.php
@@ -118,14 +118,14 @@ abstract class class_abstract_controller {
             $this->setSystemid($strSystemid);
         }
 
 
         //And keep the action
-        $this->strAction = $this->getParam("action");
+        $this->setAction($this->getParam("action"));
         //in most cases, the list is the default action if no other action was passed
-        if($this->strAction == "") {
-            $this->strAction = "list";
+        if($this->getAction() == "") {
+            $this->setAction("list");
         }
 
         //try to load the current module-name and the moduleId by reflection
         $objReflection = new class_reflection($this);
         if(!isset($this->arrModule["modul"])) {
@@ -195,11 +195,11 @@ abstract class class_abstract_controller {
      *
      * @param string $strAction
      * @return void
      */
     public final function setAction($strAction) {
-        $this->strAction = $strAction;
+        $this->strAction = htmlspecialchars(trim($strAction), ENT_QUOTES, "UTF-8", false);
     }
 
 
 
     // --- SystemID & System-Table Methods ------------------------------------------------------------------

Vulnerable code:         if($strAction == "") {
            $strAction = $this->getAction();
        }
        else {
        $this->strAction = $this->getParam("action");
        if($this->strAction == "") {
            $this->strAction = "list";
        $this->strAction = $strAction;
Fixed code:         if($strAction != "") {
        $strAction = $this->getAction();

        $this->setAction($this->getParam("action"));
        if($this->getAction() == "") {
            $this->setAction("list");
        $this->strAction = htmlspecialchars(trim($strAction), ENT_QUOTES, "UTF-8", false);