Skip to content

Commit

Permalink
Sanitize tag, item, and mod inputs (#54)
Browse files Browse the repository at this point in the history
Close #53.
  • Loading branch information
elifoster authored Aug 14, 2017
1 parent 36640fb commit b147ab9
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 10 deletions.
28 changes: 23 additions & 5 deletions OreDict.body.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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)";

Expand Down
2 changes: 2 additions & 0 deletions i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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;",
Expand Down
7 changes: 7 additions & 0 deletions special/ImportOreDict.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 30 additions & 5 deletions special/OreDictEntryManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -99,9 +116,17 @@ 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);
return ($result === false ? -2 : $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'));
Expand All @@ -113,16 +138,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) {
Expand Down

0 comments on commit b147ab9

Please sign in to comment.