From 000a5913e6e0f7f1ff17c02ae645275e3bb8b149 Mon Sep 17 00:00:00 2001 From: elifoster Date: Fri, 11 Aug 2017 13:02:13 -0700 Subject: [PATCH 1/2] Sanitize tag, item, and mod inputs Close #53. --- OreDict.body.php | 28 +++++++++++++++++++----- i18n/en.json | 2 ++ special/ImportOreDict.php | 7 ++++++ special/OreDictEntryManager.php | 38 ++++++++++++++++++++++++++++----- 4 files changed, 65 insertions(+), 10 deletions(-) diff --git a/OreDict.body.php b/OreDict.body.php index 55a0a69..d9a4e53 100755 --- a/OreDict.body.php +++ b/OreDict.body.php @@ -258,6 +258,10 @@ static public function deleteEntry($id, $user) { * @return bool|int False if it failed to add, or the new ID. */ static public function addEntry($mod, $name, $tag, $user, $params = '') { + if (!self::isStrValid($mod) || !self::isStrValid($name) || !self::isStrValid($tag)) { + return false; + } + $dbw = wfGetDB(DB_MASTER); $result = $dbw->insert( @@ -305,6 +309,14 @@ static public function addEntry($mod, $name, $tag, $user, $params = '') { return $lastInsert; } + /** + * @param $str string The string to check + * @return bool Whether this string is non-empty and does not contain invalid values. + */ + public static function isStrValid($str) { + return !empty($str) && Title::newFromText($str) !== null; + } + /** * Edits an entry based on the data given in the first parameter. * @param $update array An array essentially identical to a row. This contains the new data. @@ -322,6 +334,17 @@ static public function editEntry($update, $id, $user) { __METHOD__ ); $row = $stuff->current(); + + $tag = empty($update['tag_name']) ? $row->tag_name : $update['tag_name']; + $item = empty($update['item_name']) ? $row->item_name : $update['item_name']; + $mod = empty($update['mod_name']) ? $row->mod_name : $update['mod_name']; + $params = empty($update['grid_params']) ? $row->grid_params : $update['grid_params']; + + // Sanitize input + if (!self::isStrValid($tag) || !self::isStrValid($item) || !self::isStrValid($mod)) { + return 1; + } + $result = $dbw->update( 'ext_oredict_items', $update, @@ -333,11 +356,6 @@ static public function editEntry($update, $id, $user) { return 1; } - $tag = empty($update['tag_name']) ? $row->tag_name : $update['tag_name']; - $item = empty($update['item_name']) ? $row->item_name : $update['item_name']; - $mod = empty($update['mod_name']) ? $row->mod_name : $update['mod_name']; - $params = empty($update['grid_params']) ? $row->grid_params : $update['grid_params']; - // Prepare log vars $target = empty($mod) ? "$tag - $item" : "$tag - $item ($mod)"; diff --git a/i18n/en.json b/i18n/en.json index 3ecbfdb..a775192 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -25,6 +25,7 @@ "oredict-import-fail-nochange": "Entry already exists and no changes were made!", "oredict-import-fail-exists": "Entry already exists!", "oredict-import-fail-insert": "Insert failed!", + "oredict-import-fail-chars": "Input contains invalid values (empty or invalid characters)!", "oredict-import-comment": "Importing entries.", "oredict-tag-name": "Ore dictionary name", "oredict-item-name": "Item name", @@ -60,6 +61,7 @@ "oredict-manager-submit": "Go", "oredict-manager-fail-insert": "Insert failed!", "oredict-manager-fail-norows": "Query returned empty set (i.e., zero rows).", + "oredict-manager-fail-general": "Failed to edit entry. Double check inputs.", "log-name-oredict": "OreDict log", "log-description-oredict": "Here you can find a list of changes made to OreDict entries.", "logentry-oredict-editentry": "$1 edited entry #$15 ($7{{#if:$8| from $8}}). Changes: $4;", diff --git a/special/ImportOreDict.php b/special/ImportOreDict.php index f84ab5d..dad5c21 100644 --- a/special/ImportOreDict.php +++ b/special/ImportOreDict.php @@ -90,6 +90,13 @@ public function execute($par) { $tag = $tagName; $fItem = $itemName; $mod = $modName; + + // Sanitize inputs + if (!OreDict::isStrValid($tag) || !OreDict::isStrValid($fItem) || !OreDict::isStrValid($mod)) { + $out->addHTML($this->returnMessage(false, $this->msg('oredict-import-fail-chars')->text())); + continue; + } + $item = $stuff->current(); // Prepare log vars diff --git a/special/OreDictEntryManager.php b/special/OreDictEntryManager.php index f1db8ce..d1bf55f 100644 --- a/special/OreDictEntryManager.php +++ b/special/OreDictEntryManager.php @@ -68,7 +68,24 @@ public function execute($par) { return; } - $this->updateEntry($opts); + $result = $this->updateEntry($opts); + // It's obvious if a deletion was successful + if ($opts->getValue('delete') != 1) { + switch ($result) { + case 0: { + // Success: no op + break; + } + case 1: { + $out->addWikiText($this->msg('oredict-manager-fail-general')->text()); + break; + } + case 2: { + $out->addWikiText($this->msg('oredict-import-fail-nochange')->text()); + break; + } + } + } } // Load data @@ -99,9 +116,20 @@ private function createEntry(FormOptions $opts) { return -2; } - return OreDict::addEntry($mod, $item, $tag, $this->getUser(), $params); + $result = OreDict::addEntry($mod, $item, $tag, $this->getUser(), $params); + if ($result == false) { + return -2; + } + return $result; } + /** + * @param FormOptions $opts + * @return bool|int False if it was a deletion or there was nothing to update. These cases are not important for the + * purposes of this special page. + * Returns the integer returned by OreDict#editEntry denoting success of the update. This is + * actually useful. + */ private function updateEntry(FormOptions $opts) { $dbw = wfGetDB(DB_MASTER); $entryId = intval($opts->getValue('entry_id')); @@ -113,16 +141,16 @@ private function updateEntry(FormOptions $opts) { 'grid_params' => $opts->getValue('grid_params'), ); if ($stuff->numRows() == 0) { - return; + return false; } // Try to delete before doing any other processing if ($opts->getValue('delete') == 1) { OreDict::deleteEntry($entryId, $this->getUser()); - return; + return false; } - OreDict::editEntry($ary, $entryId, $this->getUser()); + return OreDict::editEntry($ary, $entryId, $this->getUser()); } private function outputUpdateForm(stdClass $opts = NULL) { From 4820122412e839afa97135a4111c3c4bfd70b224 Mon Sep 17 00:00:00 2001 From: elifoster Date: Mon, 14 Aug 2017 10:37:02 -0700 Subject: [PATCH 2/2] Fix loose false check, use ternary --- special/OreDictEntryManager.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/special/OreDictEntryManager.php b/special/OreDictEntryManager.php index d1bf55f..2a61035 100644 --- a/special/OreDictEntryManager.php +++ b/special/OreDictEntryManager.php @@ -117,10 +117,7 @@ private function createEntry(FormOptions $opts) { } $result = OreDict::addEntry($mod, $item, $tag, $this->getUser(), $params); - if ($result == false) { - return -2; - } - return $result; + return ($result === false ? -2 : $result); } /**