Skip to content

Commit

Permalink
Merge pull request from GHSA-pqfv-4pvr-55r4
Browse files Browse the repository at this point in the history
validate itemtype before send data from getDropdownValue
move idor functions to Session, and generalize idor checks to all ajax dropdowns
add idor tokens to search engine
idor token for getDropdownUsers now include right param
improve idor mecanism to have more params to check
add tests
protect from idor also comments
switch ajax/comments.php to itemtype param instead table param
  • Loading branch information
orthagh committed Nov 25, 2020
1 parent 5cfa385 commit e0d6a24
Show file tree
Hide file tree
Showing 16 changed files with 298 additions and 176 deletions.
2 changes: 1 addition & 1 deletion .composer-require-checker.config.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"DB", "DBSlave",

"// GLPI base constants (they are not detected as they are dynamically declared)",
"GLPI_AJAX_DASHBOARD", "GLPI_CALDAV_IMPORT_STATE", "GLPI_CACHE_DIR", "GLPI_CRON_DIR", "GLPI_CSRF_EXPIRES", "GLPI_CSRF_MAX_TOKENS", "GLPI_DEMO_MODE", "GLPI_DOC_DIR", "GLPI_DUMP_DIR", "GLPI_FORCE_EMPTY_SQL_MODE", "GLPI_GRAPH_DIR", "GLPI_INSTALL_MODE", "GLPI_LOCAL_I18N_DIR", "GLPI_LOCK_DIR", "GLPI_LOG_DIR", "GLPI_MARKETPLACE_DIR", "GLPI_MARKETPLACE_PLUGINS_API_URI", "GLPI_MARKETPLACE_PRERELEASES", "GLPI_NETWORK_REGISTRATION_API_URL", "GLPI_NETWORK_MAIL", "GLPI_NETWORK_SERVICES", "GLPI_PICTURE_DIR", "GLPI_PLUGIN_DOC_DIR", "GLPI_RSS_DIR", "GLPI_SESSION_DIR", "GLPI_TELEMETRY_URI", "GLPI_TMP_DIR", "GLPI_UPLOAD_DIR", "GLPI_USE_CSRF_CHECK", "GLPI_USER_AGENT_EXTRA_COMMENTS",
"GLPI_AJAX_DASHBOARD", "GLPI_CALDAV_IMPORT_STATE", "GLPI_CACHE_DIR", "GLPI_CRON_DIR", "GLPI_CSRF_EXPIRES", "GLPI_CSRF_MAX_TOKENS", "GLPI_USE_IDOR_CHECK", "GLPI_IDOR_EXPIRES", "GLPI_DEMO_MODE", "GLPI_DOC_DIR", "GLPI_DUMP_DIR", "GLPI_FORCE_EMPTY_SQL_MODE", "GLPI_GRAPH_DIR", "GLPI_INSTALL_MODE", "GLPI_LOCAL_I18N_DIR", "GLPI_LOCK_DIR", "GLPI_LOG_DIR", "GLPI_MARKETPLACE_DIR", "GLPI_MARKETPLACE_PLUGINS_API_URI", "GLPI_MARKETPLACE_PRERELEASES", "GLPI_NETWORK_REGISTRATION_API_URL", "GLPI_NETWORK_MAIL", "GLPI_NETWORK_SERVICES", "GLPI_PICTURE_DIR", "GLPI_PLUGIN_DOC_DIR", "GLPI_RSS_DIR", "GLPI_SESSION_DIR", "GLPI_TELEMETRY_URI", "GLPI_TMP_DIR", "GLPI_UPLOAD_DIR", "GLPI_USE_CSRF_CHECK", "GLPI_USER_AGENT_EXTRA_COMMENTS",

"// GLPI optionnal constants",
"GLPI_FORCE_MAIL", "GLPI_LOG_LVL",
Expand Down
28 changes: 21 additions & 7 deletions ajax/comments.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,22 @@

Session::checkLoginUser();

if (isset($_POST["table"])
// depreciation behavior
if (!isset($_POST["itemtype"]) && isset($_POST['table'])
&& $DB->tableExists($_POST['table'])) {
Toolbox::deprecated();
$_POST["itemtype"] = getItemTypeForTable($_POST['table']);
}

if (isset($_POST["itemtype"])
&& isset($_POST["value"])) {
// Security
if (!$DB->tableExists($_POST['table'])) {
if (!is_subclass_of($_POST["itemtype"], "CommonDBTM")) {
exit();
}

switch ($_POST["table"]) {
case "glpi_users" :
switch ($_POST["itemtype"]) {
case User::getType() :
if ($_POST['value'] == 0) {
$tmpname = [
'link' => $CFG_GLPI['root_doc']."/front/user.php",
Expand Down Expand Up @@ -79,15 +86,22 @@

default :
if ($_POST["value"] > 0) {
$tmpname = Dropdown::getDropdownName($_POST["table"], $_POST["value"], 1);
if (!Session::validateIDOR([
'itemtype' => $_POST['itemtype'],
'_idor_token' => $_POST['_idor_token'] ?? ""
])) {
exit();
}

$table = getTableForItemType($_POST['itemtype']);
$tmpname = Dropdown::getDropdownName($table, $_POST["value"], 1);
if (is_array($tmpname) && isset($tmpname["comment"])) {
echo $tmpname["comment"];
}
if (isset($_POST['withlink'])) {
$itemtype = getItemTypeForTable($_POST["table"]);
echo "<script type='text/javascript' >\n";
echo Html::jsGetElementbyID($_POST['withlink']).".
attr('href', '".$itemtype::getFormURLWithID($_POST["value"])."');";
attr('href', '".$_POST['itemtype']::getFormURLWithID($_POST["value"])."');";
echo "</script>\n";
}
}
Expand Down
13 changes: 8 additions & 5 deletions ajax/dropdownAllItems.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,14 @@

$field_id = Html::cleanId("dropdown_".$_POST["name"].$rand);

$p = ['value' => 0,
'valuename' => Dropdown::EMPTY_VALUE,
'itemtype' => $_POST["idtable"],
'display_emptychoice' => true,
'displaywith' => ['otherserial', 'serial']];
$p = [
'value' => 0,
'valuename' => Dropdown::EMPTY_VALUE,
'itemtype' => $_POST["idtable"],
'display_emptychoice' => true,
'displaywith' => ['otherserial', 'serial'],
'_idor_token' => Session::getNewIDORToken($_POST["idtable"]),
];
if (isset($_POST['value'])) {
$p['value'] = $_POST['value'];
}
Expand Down
15 changes: 9 additions & 6 deletions ajax/dropdownTrackingDeviceType.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,15 @@
}
echo "<br>";
$field_id = Html::cleanId("dropdown_".$_POST['myname'].$rand);
$p = ['itemtype' => $itemtype,
'entity_restrict' => $_POST['entity_restrict'],
'table' => $table,
'multiple' => $_POST["multiple"],
'myname' => $_POST["myname"],
'rand' => $_POST["rand"]];
$p = [
'itemtype' => $itemtype,
'entity_restrict' => $_POST['entity_restrict'],
'table' => $table,
'multiple' => $_POST["multiple"],
'myname' => $_POST["myname"],
'rand' => $_POST["rand"],
'_idor_token' => Session::getNewIDORToken($itemtype),
];

if (isset($_POST["used"]) && !empty($_POST["used"])) {
if (isset($_POST["used"][$itemtype])) {
Expand Down
6 changes: 6 additions & 0 deletions ajax/kblink.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@

if (isset($_POST['withlink'])) {
$itemtype = getItemTypeForTable($_POST["table"]);
if (!Session::validateIDOR([
'itemtype' => $itemtype,
'_idor_token' => $_POST['_idor_token'] ?? ""
])) {
exit();
}
$item = new $itemtype;
$item->getFromDB(intval($_POST["value"]));
echo '&nbsp;'.$item->getLinks();
Expand Down
4 changes: 4 additions & 0 deletions ajax/search.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
die;
}

if (!Session::validateIDOR($_REQUEST)) {
die;
}

switch ($_REQUEST['action']) {
case "display_criteria":
Search::displayCriteria($_REQUEST);
Expand Down
50 changes: 0 additions & 50 deletions front/impact.php

This file was deleted.

2 changes: 2 additions & 0 deletions inc/based_config.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
'GLPI_USE_CSRF_CHECK' => '1',
'GLPI_CSRF_EXPIRES' => '7200',
'GLPI_CSRF_MAX_TOKENS' => '100',
'GLPI_USE_IDOR_CHECK' => '1',
'GLPI_IDOR_EXPIRES' => '7200',

// Constants related to GLPI Project / GLPI Network external services
'GLPI_TELEMETRY_URI' => 'https://telemetry.glpi-project.org', // Telemetry project URL
Expand Down
13 changes: 8 additions & 5 deletions inc/computer_item.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -676,11 +676,14 @@ static function dropdownConnect($itemtype, $fromtype, $myname, $entity_restrict
$rand = mt_rand();

$field_id = Html::cleanId("dropdown_".$myname.$rand);
$param = ['entity_restrict' => $entity_restrict,
'fromtype' => $fromtype,
'itemtype' => $itemtype,
'onlyglobal' => $onlyglobal,
'used' => $used];
$param = [
'entity_restrict' => $entity_restrict,
'fromtype' => $fromtype,
'itemtype' => $itemtype,
'onlyglobal' => $onlyglobal,
'used' => $used,
'_idor_token' => Session::getNewIDORToken($itemtype),
];

echo Html::jsAjaxDropdown($myname, $field_id,
$CFG_GLPI['root_doc']."/ajax/getDropdownConnect.php",
Expand Down
32 changes: 29 additions & 3 deletions inc/dropdown.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ static function show($itemtype, $options = []) {
'on_change' => $params['on_change'],
'permit_select_parent' => $params['permit_select_parent'],
'specific_tags' => $params['specific_tags'],
'_idor_token' => Session::getNewIDORToken($itemtype),
];

$output = "<span class='no-wrap'>";
Expand Down Expand Up @@ -235,8 +236,11 @@ static function show($itemtype, $options = []) {
$output .= "<span class='fa fa-globe-americas pointer' title='".__s('Display on map')."' onclick='showMapForLocation(this)' data-fid='$field_id'></span>";
}

$paramscomment = ['value' => '__VALUE__',
'table' => $table];
$paramscomment = [
'value' => '__VALUE__',
'itemtype' => $itemtype,
'_idor_token' => Session::getNewIDORToken($itemtype)
];
if ($item->isField('knowbaseitemcategories_id')
&& Session::haveRight('knowbase', READ)) {

Expand Down Expand Up @@ -268,6 +272,7 @@ static function show($itemtype, $options = []) {
return $output;
}


/**
* Add new condition
*
Expand Down Expand Up @@ -1466,7 +1471,7 @@ static function showSelectItemFromItemtypes(array $options = []) {
'name' => $params['items_id_name'],
'entity_restrict' => $params['entity_restrict'],
'showItemSpecificity' => $params['showItemSpecificity'],
'rand' => $params['rand'],
'rand' => $params['rand']
];

// manage condition
Expand Down Expand Up @@ -2302,10 +2307,16 @@ public static function getDropdownValue($post, $json = true) {
$post['entity_restrict'] = $_SESSION['glpiactiveentities'];
}

// check if asked itemtype is the one originaly requested by the form
if (!Session::validateIDOR($post)) {
return;
}

// Security
if (!($item = getItemForItemtype($post['itemtype']))) {
return;
}

$table = $item->getTable();
$datas = [];

Expand Down Expand Up @@ -3051,6 +3062,11 @@ public static function getDropdownValue($post, $json = true) {
public static function getDropdownConnect($post, $json = true) {
global $DB, $CFG_GLPI;

// check if asked itemtype is the one originaly requested by the form
if (!Session::validateIDOR($post)) {
return;
}

if (!isset($post['fromtype']) || !($fromitem = getItemForItemtype($post['fromtype']))) {
return;
}
Expand Down Expand Up @@ -3253,6 +3269,11 @@ public static function getDropdownFindNum($post, $json = true) {

$itemtypeisplugin = isPluginItemType($post['itemtype']);

// check if asked itemtype is the one originaly requested by the form
if (!Session::validateIDOR($post)) {
return;
}

if (!$item = getItemForItemtype($post['itemtype'])) {
return;
}
Expand Down Expand Up @@ -3637,6 +3658,11 @@ public static function getDropdownNumber($post, $json = true) {
public static function getDropdownUsers($post, $json = true) {
global $CFG_GLPI;

// check if asked itemtype is the one originaly requested by the form
if (!Session::validateIDOR($post + ['itemtype' => 'User', 'right' => ($post['right'] ?? "")])) {
return;
}

if (!isset($post['right'])) {
$post['right'] = "all";
}
Expand Down
64 changes: 1 addition & 63 deletions inc/impact.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -1612,76 +1612,14 @@ public static function getEdgeID(
}
}

/**
* Print the form for the global impact page
*/
public static function printImpactForm() {
global $CFG_GLPI;
$rand = mt_rand();

echo "<form name=\"item\" action=\"{$_SERVER['PHP_SELF']}\" method=\"GET\">";

echo '<table class="tab_cadre_fixe" style="width:30%">';

// First row: header
echo "<tr>";
echo "<th colspan=\"2\">" . self::getTypeName() . "</th>";
echo "</tr>";

// Second row: itemtype field
echo "<tr>";
echo "<td width=\"40%\"> <label>" . __('Item type') . "</label> </td>";
echo "<td>";
Dropdown::showItemTypes(
'type',
self::getEnabledItemtypes(),
[
'value' => null,
'width' => '100%',
'emptylabel' => Dropdown::EMPTY_VALUE,
'rand' => $rand
]
);
echo "</td>";
echo "</tr>";

// Third row: items_id field
echo "<tr>";
echo "<td> <label>" . _n('Item', 'Items', 1) . "</label> </td>";
echo "<td>";
Ajax::updateItemOnSelectEvent("dropdown_type$rand", "form_results",
$CFG_GLPI["root_doc"] . "/ajax/dropdownTrackingDeviceType.php",
[
'itemtype' => '__VALUE__',
'entity_restrict' => 0,
'multiple' => 1,
'admin' => 1,
'rand' => $rand,
'myname' => "id",
]
);
echo "<span id='form_results'>\n";
echo "</span>\n";
echo "</td>";
echo "</tr>";

// Fourth row: submit
echo "<tr><td colspan=\"2\" style=\"text-align:center\">";
echo Html::submit(__("Show impact analysis"));
echo "</td></tr>";

echo "</table>";
echo "<br><br>";
Html::closeForm();
}

/**
* Clean impact records for a given item that has been purged form the db
*
* @param CommonDBTM $item The item being purged
*/
public static function clean(\CommonDBTM $item) {
global $DB, $CFG_GLPI;
global $DB;

// Skip if not a valid impact type
if (!self::isEnabled($item::getType())) {
Expand Down
7 changes: 5 additions & 2 deletions inc/netpoint.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,11 @@ static function dropdownNetpoint($myname, $value = 0, $locations_id = -1, $displ
$item->getFormURL());

}
$paramscomment = ['value' => '__VALUE__',
'table' => "glpi_netpoints"];
$paramscomment = [
'value' => '__VALUE__',
'itemtype' => Netpoint::getType(),
'_idor_token' => Session::getNewIDORToken("Netpoint")
];
echo Ajax::updateItemOnSelectEvent($field_id, $comment_id,
$CFG_GLPI["root_doc"]."/ajax/comments.php",
$paramscomment, false);
Expand Down
Loading

0 comments on commit e0d6a24

Please sign in to comment.