From 5b87b12e64d88898279bd830ee3cccd30a373b96 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sun, 16 Feb 2020 18:06:25 +0100 Subject: [PATCH] FIX Vulnerability reported by code16 --- htdocs/admin/dict.php | 50 +++++++++++++++++++++++-------- htdocs/admin/mails_templates.php | 51 ++++++++++++++++++++++---------- htdocs/filefunc.inc.php | 4 ++- 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/htdocs/admin/dict.php b/htdocs/admin/dict.php index d634f547ff9..314a4e61954 100644 --- a/htdocs/admin/dict.php +++ b/htdocs/admin/dict.php @@ -753,20 +753,33 @@ if (GETPOST('actionadd') || GETPOST('actionmodify')) $i = 0; foreach ($listfieldinsert as $f => $value) { + $keycode = $listfieldvalue[$i]; + if (empty($keycode)) $keycode = $value; + if ($value == 'price' || preg_match('/^amount/i', $value) || $value == 'taux') { - $_POST[$listfieldvalue[$i]] = price2num($_POST[$listfieldvalue[$i]], 'MU'); + $_POST[$keycode] = price2num($_POST[$keycode], 'MU'); } elseif ($value == 'entity') { - $_POST[$listfieldvalue[$i]] = getEntity($tabname[$id]); + $_POST[$keycode] = getEntity($tabname[$id]); } + if ($i) $sql .= ","; - if ($listfieldvalue[$i] == 'sortorder') // For column name 'sortorder', we use the field name 'position' + if ($keycode == 'sortorder') // For column name 'sortorder', we use the field name 'position' { - $sql .= "'".(int) $db->escape(GETPOST('position'))."'"; + $sql .= "'".(int) GETPOST('position', 'int'); } - elseif ($_POST[$listfieldvalue[$i]] == '' && !($listfieldvalue[$i] == 'code' && $id == 10)) $sql .= "null"; // For vat, we want/accept code = '' - else $sql .= "'".$db->escape(GETPOST($listfieldvalue[$i], 'nohtml'))."'"; + elseif ($_POST[$keycode] == '' && !($keycode == 'code' && $id == 10)) $sql .= "null"; // For vat, we want/accept code = '' + elseif ($keycode == 'content') { + $sql .= "'".$db->escape(GETPOST($keycode, 'restricthtml'))."'"; + } + elseif (in_array($keycode, array('joinfile', 'private', 'position', 'scale'))) { + $sql .= (int) GETPOST($keycode, 'int'); + } + else { + $sql .= "'".$db->escape(GETPOST($keycode, 'nohtml'))."'"; + } + $i++; } $sql .= ",1)"; @@ -806,23 +819,36 @@ if (GETPOST('actionadd') || GETPOST('actionmodify')) $i = 0; foreach ($listfieldmodify as $field) { + $keycode = $listfieldvalue[$i]; + if (empty($keycode)) $keycode = $field; + if ($field == 'price' || preg_match('/^amount/i', $field) || $field == 'taux') { - $_POST[$listfieldvalue[$i]] = price2num($_POST[$listfieldvalue[$i]], 'MU'); + $_POST[$keycode] = price2num($_POST[$keycode], 'MU'); } elseif ($field == 'entity') { - $_POST[$listfieldvalue[$i]] = getEntity($tabname[$id]); + $_POST[$keycode] = getEntity($tabname[$id]); } + if ($i) $sql .= ","; $sql .= $field."="; if ($listfieldvalue[$i] == 'sortorder') // For column name 'sortorder', we use the field name 'position' { - $sql .= "'".(int) $db->escape($_POST['position'])."'"; + $sql .= (int) GETPOST('position', 'int'); } - elseif ($_POST[$listfieldvalue[$i]] == '' && !($listfieldvalue[$i] == 'code' && $id == 10)) $sql .= "null"; // For vat, we want/accept code = '' - else $sql .= "'".$db->escape($_POST[$listfieldvalue[$i]])."'"; + elseif ($_POST[$keycode] == '' && !($keycode == 'code' && $id == 10)) $sql .= "null"; // For vat, we want/accept code = '' + elseif ($keycode == 'content') { + $sql .= "'".$db->escape(GETPOST($keycode, 'restricthtml'))."'"; + } + elseif (in_array($keycode, array('private', 'position', 'scale'))) { + $sql .= (int) GETPOST($keycode, 'int'); + } + else { + $sql .= "'".$db->escape(GETPOST($keycode, 'nohtml'))."'"; + } + $i++; } - $sql .= " WHERE ".$rowidcol." = '".$db->escape($rowid)."'"; + $sql .= " WHERE ".$rowidcol." = ".(int) $db->escape($rowid); if (in_array('entity', $listfieldmodify)) $sql .= " AND entity = '".getEntity($tabname[$id])."'"; dol_syslog("actionmodify", LOG_DEBUG); diff --git a/htdocs/admin/mails_templates.php b/htdocs/admin/mails_templates.php index bb5fd72ec29..3aa5927af63 100644 --- a/htdocs/admin/mails_templates.php +++ b/htdocs/admin/mails_templates.php @@ -58,6 +58,8 @@ $search_topic = GETPOST('search_topic', 'alpha'); if (!empty($user->socid)) accessforbidden(); +$acts = array(); +$actl = array(); $acts[0] = "activate"; $acts[1] = "disable"; $actl[0] = img_picto($langs->trans("Disabled"), 'switch_off'); @@ -261,28 +263,39 @@ if (empty($reshook)) $i = 0; foreach ($listfieldinsert as $f => $value) { - //var_dump($i.' - '.$listfieldvalue[$i].' - '.$_POST[$listfieldvalue[$i]].' - '.$value); $keycode = $listfieldvalue[$i]; - if ($value == 'label') $_POST[$keycode] = dol_escape_htmltag($_POST[$keycode]); if ($value == 'lang') $keycode = 'langcode'; + if (empty($keycode)) $keycode = $value; + if ($value == 'entity') $_POST[$keycode] = $conf->entity; - if ($i) $sql .= ","; if ($value == 'fk_user' && !($_POST[$keycode] > 0)) $_POST[$keycode] = ''; if ($value == 'private' && !is_numeric($_POST[$keycode])) $_POST[$keycode] = '0'; if ($value == 'position' && !is_numeric($_POST[$keycode])) $_POST[$keycode] = '1'; - if ($_POST[$keycode] == '' && $keycode != 'langcode') $sql .= "null"; // lang must be '' if not defined so the unique key that include lang will work - elseif ($_POST[$keycode] == '0' && $keycode == 'langcode') $sql .= "''"; // lang must be '' if not defined so the unique key that include lang will work - else $sql .= "'".$db->escape($_POST[$keycode])."'"; + //var_dump($keycode.' '.$value); + + if ($i) $sql .= ", "; + if (GETPOST($keycode) == '' && $keycode != 'langcode') $sql .= "null"; // langcode must be '' if not defined so the unique key that include lang will work + elseif (GETPOST($keycode) == '0' && $keycode == 'langcode') $sql .= "''"; // langcode must be '' if not defined so the unique key that include lang will work + elseif ($keycode == 'content') { + $sql .= "'".$db->escape(GETPOST($keycode, 'restricthtml'))."'"; + } + elseif (in_array($keycode, array('joinfile', 'private', 'position', 'scale'))) { + $sql .= (int) GETPOST($keycode, 'int'); + } + else { + $sql .= "'".$db->escape(GETPOST($keycode, 'nohtml'))."'"; + } + $i++; } - $sql .= ",1)"; + $sql .= ", 1)"; dol_syslog("actionadd", LOG_DEBUG); $result = $db->query($sql); if ($result) // Add is ok { setEventMessages($langs->transnoentities("RecordSaved"), null, 'mesgs'); - $_POST = array('id'=>$id); // Clean $_POST array, we keep only + $_POST = array('id'=>$id); // Clean $_POST array, we keep only id } else { @@ -308,6 +321,7 @@ if (empty($reshook)) { $keycode = $listfieldvalue[$i]; if ($field == 'lang') $keycode = 'langcode'; + if (empty($keycode)) $keycode = $field; if ($field == 'fk_user' && !($_POST['fk_user'] > 0)) $_POST['fk_user'] = ''; if ($field == 'topic') $_POST['topic'] = $_POST['topic-'.$rowid]; @@ -315,15 +329,22 @@ if (empty($reshook)) if ($field == 'content') $_POST['content'] = $_POST['content-'.$rowid]; if ($field == 'content_lines') $_POST['content_lines'] = $_POST['content_lines-'.$rowid]; if ($field == 'entity') $_POST[$keycode] = $conf->entity; - if ($i) $sql .= ","; + + if ($i) $sql .= ", "; $sql .= $field."="; - //print $keycode.' - '.$_POST[$keycode].'
'; - if ($_POST[$keycode] == '' || ($keycode != 'langcode' && $keycode != 'position' && $keycode != 'private' && empty($_POST[$keycode]))) $sql .= "null"; // lang must be '' if not defined so the unique key that include lang will work - elseif ($_POST[$keycode] == '0' && $keycode == 'langcode') $sql .= "''"; // lang must be '' if not defined so the unique key that include lang will work - elseif ($keycode == 'private') $sql .= ((int) $_POST[$keycode]); // private must be 0 or 1 - elseif ($keycode == 'position') $sql .= ((int) $_POST[$keycode]); - else $sql .= "'".$db->escape($_POST[$keycode])."'"; + if (GETPOST($keycode) == '' || ($keycode != 'langcode' && $keycode != 'position' && $keycode != 'private' && ! GETPOST($keycode))) $sql .= "null"; // langcode,... must be '' if not defined so the unique key that include lang will work + elseif (GETPOST($keycode) == '0' && $keycode == 'langcode') $sql .= "''"; // langcode must be '' if not defined so the unique key that include lang will work + elseif ($keycode == 'content') { + $sql .= "'".$db->escape(GETPOST($keycode, 'restricthtml'))."'"; + } + elseif (in_array($keycode, array('joinfile', 'private', 'position', 'scale'))) { + $sql .= (int) GETPOST($keycode, 'int'); + } + else { + $sql .= "'".$db->escape(GETPOST($keycode, 'nohtml'))."'"; + } + $i++; } $sql .= " WHERE ".$rowidcol." = '".$rowid."'"; diff --git a/htdocs/filefunc.inc.php b/htdocs/filefunc.inc.php index a498c933d4b..01245a466cc 100644 --- a/htdocs/filefunc.inc.php +++ b/htdocs/filefunc.inc.php @@ -165,7 +165,9 @@ if (! defined('NOCSRFCHECK') && empty($dolibarr_nocsrfcheck)) if ($csrfattack) { //print 'NOCSRFCHECK='.defined('NOCSRFCHECK').' REQUEST_METHOD='.$_SERVER['REQUEST_METHOD'].' HTTP_HOST='.$_SERVER['HTTP_HOST'].' HTTP_REFERER='.$_SERVER['HTTP_REFERER']; - print "Access refused by CSRF protection in main.inc.php. Referer of form (".$_SERVER['HTTP_REFERER'].") is outside the server that serve this page (with method = ".$_SERVER['REQUEST_METHOD'].").\n"; + + // Note: We can't use dol_escape_htmltag here to escape output because lib functions.lib.ph is not yet loaded. + print "Access refused by CSRF protection in main.inc.php. Referer of form (".htmlentities($_SERVER['HTTP_REFERER'], ENT_COMPAT, 'UTF-8').") is outside the server that serve this page (with method = ".htmlentities($_SERVER['REQUEST_METHOD'], ENT_COMPAT, 'UTF-8').").\n"; print "If you access your server behind a proxy using url rewriting, you might check that all HTTP headers are propagated (or add the line \$dolibarr_nocsrfcheck=1 into your conf.php file to remove this security check).\n"; die; }