From 7cdfc3ca657b1cbdaf9d991d4b6c5fa0996efbc3 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sun, 16 Feb 2020 12:55:32 +0100 Subject: [PATCH 1/3] FIX HTML Injection --- htdocs/user/param_ihm.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/htdocs/user/param_ihm.php b/htdocs/user/param_ihm.php index dba57f3954b..8dfe8480722 100644 --- a/htdocs/user/param_ihm.php +++ b/htdocs/user/param_ihm.php @@ -89,49 +89,49 @@ if (empty($reshook)) { $tabparam = array(); if (GETPOST("check_MAIN_LANDING_PAGE") == "on") { - $tabparam["MAIN_LANDING_PAGE"] = $_POST["MAIN_LANDING_PAGE"]; + $tabparam["MAIN_LANDING_PAGE"] = GETPOST("MAIN_LANDING_PAGE", 'alphanohtml'); } else { $tabparam["MAIN_LANDING_PAGE"] = ''; } if (GETPOST("check_MAIN_LANG_DEFAULT") == "on") { - $tabparam["MAIN_LANG_DEFAULT"] = $_POST["main_lang_default"]; + $tabparam["MAIN_LANG_DEFAULT"] = GETPOST("main_lang_default", 'aZ09'); } else { $tabparam["MAIN_LANG_DEFAULT"] = ''; } if (GETPOST("check_SIZE_LISTE_LIMIT") == "on") { - $tabparam["MAIN_SIZE_LISTE_LIMIT"] = $_POST["main_size_liste_limit"]; + $tabparam["MAIN_SIZE_LISTE_LIMIT"] = GETPOST("main_size_liste_limit", 'int'); } else { $tabparam["MAIN_SIZE_LISTE_LIMIT"] = ''; } if (GETPOST("check_AGENDA_DEFAULT_VIEW") == "on") { - $tabparam["AGENDA_DEFAULT_VIEW"] = $_POST["AGENDA_DEFAULT_VIEW"]; + $tabparam["AGENDA_DEFAULT_VIEW"] = GETPOST("AGENDA_DEFAULT_VIEW", 'aZ09'); } else { $tabparam["AGENDA_DEFAULT_VIEW"] = ''; } if (GETPOST("check_MAIN_THEME") == "on") { - $tabparam["MAIN_THEME"] = $_POST["main_theme"]; + $tabparam["MAIN_THEME"] = GETPOST('main_theme', 'aZ09'); } else { $tabparam["MAIN_THEME"] = ''; } - $val = (implode(',', (colorStringToArray(GETPOST('THEME_ELDY_TOPMENU_BACK1'), array())))); + $val = (implode(',', (colorStringToArray(GETPOST('THEME_ELDY_TOPMENU_BACK1', 'alphanohtml'), array())))); if ($val == '') { $tabparam['THEME_ELDY_TOPMENU_BACK1'] = ''; } else { $tabparam['THEME_ELDY_TOPMENU_BACK1'] = join(',', - colorStringToArray(GETPOST('THEME_ELDY_TOPMENU_BACK1'), array())); + colorStringToArray(GETPOST('THEME_ELDY_TOPMENU_BACK1', 'alphanohtml'), array())); } - $val = (implode(',', (colorStringToArray(GETPOST('THEME_ELDY_BACKTITLE1'), array())))); + $val = (implode(',', (colorStringToArray(GETPOST('THEME_ELDY_BACKTITLE1', 'alphanohtml'), array())))); if ($val == '') { $tabparam['THEME_ELDY_BACKTITLE1'] = ''; } else { $tabparam['THEME_ELDY_BACKTITLE1'] = join(',', - colorStringToArray(GETPOST('THEME_ELDY_BACKTITLE1'), array())); + colorStringToArray(GETPOST('THEME_ELDY_BACKTITLE1', 'alphanohtml'), array())); } if (GETPOST('check_THEME_ELDY_USE_HOVER') == 'on') { @@ -153,7 +153,7 @@ if (empty($reshook)) { } if (GETPOST('MAIN_OPTIMIZEFORCOLORBLIND')) { - $tabparam["MAIN_OPTIMIZEFORCOLORBLIND"] = GETPOST('MAIN_OPTIMIZEFORCOLORBLIND'); + $tabparam["MAIN_OPTIMIZEFORCOLORBLIND"] = GETPOST('MAIN_OPTIMIZEFORCOLORBLIND', 'aZ09'); } else { $tabparam["MAIN_OPTIMIZEFORCOLORBLIND"] = 0; } From 5b87b12e64d88898279bd830ee3cccd30a373b96 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sun, 16 Feb 2020 18:06:25 +0100 Subject: [PATCH 2/3] 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; } From ac7a077c77b6c6b922225ec0d47120b886898196 Mon Sep 17 00:00:00 2001 From: Laurent Destailleur Date: Sun, 16 Feb 2020 18:14:10 +0100 Subject: [PATCH 3/3] FIX Vulnerability in module from modulebuilder. Only fields with type html can contains HTML. --- htdocs/core/actions_addupdatedelete.inc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htdocs/core/actions_addupdatedelete.inc.php b/htdocs/core/actions_addupdatedelete.inc.php index 59606ee65b5..ebc8e48d742 100644 --- a/htdocs/core/actions_addupdatedelete.inc.php +++ b/htdocs/core/actions_addupdatedelete.inc.php @@ -76,7 +76,7 @@ if ($action == 'add' && !empty($permissiontoadd)) } elseif (preg_match('/^(integer|price|real|double)/', $object->fields[$key]['type'])) { $value = price2num(GETPOST($key, 'none')); // To fix decimal separator according to lang setup } else { - $value = GETPOST($key, 'alpha'); + $value = GETPOST($key, 'alphanohtml'); } if (preg_match('/^integer:/i', $object->fields[$key]['type']) && $value == '-1') $value = ''; // This is an implicit foreign key field if (!empty($object->fields[$key]['foreignkey']) && $value == '-1') $value = ''; // This is an explicit foreign key field