Html entities use now HTML5. Enhance the Dolibarr WAF. More PHPUnit

tests.
This commit is contained in:
Laurent Destailleur 2020-10-15 19:36:08 +02:00
parent 844c4dec4d
commit 5b37ff0bfd
16 changed files with 127 additions and 47 deletions

View File

@ -224,8 +224,8 @@ class Mailing extends CommonObject
$this->title = $obj->title;
$this->sujet = $obj->sujet;
if (!empty($conf->global->FCKEDITOR_ENABLE_MAILING) && dol_textishtml(dol_html_entity_decode($obj->body, ENT_COMPAT | ENT_HTML401))) {
$this->body = dol_html_entity_decode($obj->body, ENT_COMPAT | ENT_HTML401);
if (!empty($conf->global->FCKEDITOR_ENABLE_MAILING) && dol_textishtml(dol_html_entity_decode($obj->body, ENT_COMPAT|ENT_HTML5))) {
$this->body = dol_html_entity_decode($obj->body, ENT_COMPAT|ENT_HTML5);
} else {
$this->body = $obj->body;
}

View File

@ -33,7 +33,7 @@ if ($action == 'setnote_public' && !empty($permissionnote) && !GETPOST('cancel',
if (empty($action) || !is_object($object) || empty($id)) dol_print_error('', 'Include of actions_setnotes.inc.php was done but required variable was not set before');
if (empty($object->id)) $object->fetch($id); // Fetch may not be already done
$result_update = $object->update_note(dol_html_entity_decode(GETPOST('note_public', 'restricthtml'), ENT_QUOTES, 'UTF-8', 1), '_public');
$result_update = $object->update_note(dol_html_entity_decode(GETPOST('note_public', 'restricthtml'), ENT_QUOTES|ENT_HTML5, 'UTF-8', 1), '_public');
if ($result_update < 0) setEventMessages($object->error, $object->errors, 'errors');
elseif (in_array($object->table_element, array('supplier_proposal', 'propal', 'commande_fournisseur', 'commande', 'facture_fourn', 'facture')))
@ -63,6 +63,6 @@ if ($action == 'setnote_public' && !empty($permissionnote) && !GETPOST('cancel',
// Set public note
if (empty($action) || !is_object($object) || empty($id)) dol_print_error('', 'Include of actions_setnotes.inc.php was done but required variable was not set before');
if (empty($object->id)) $object->fetch($id); // Fetch may not be already done
$result = $object->update_note(dol_html_entity_decode(GETPOST('note_private', 'restricthtml'), ENT_QUOTES), '_private');
$result = $object->update_note(dol_html_entity_decode(GETPOST('note_private', 'restricthtml'), ENT_QUOTES|ENT_HTML5), '_private');
if ($result < 0) setEventMessages($object->error, $object->errors, 'errors');
}

View File

@ -264,6 +264,7 @@ class Form
elseif (preg_match('/^(amount|numeric)/', $typeofdata)) $ret .= ($value != '' ? price($value, '', $langs, 0, -1, -1, $conf->currency) : '');
elseif (preg_match('/^text/', $typeofdata) || preg_match('/^note/', $typeofdata)) $ret .= dol_htmlentitiesbr($value);
elseif (preg_match('/^safehtmlstring/', $typeofdata)) $ret .= dol_string_onlythesehtmltags($value);
elseif (preg_match('/^restricthtml/', $typeofdata)) $ret .= dol_string_onlythesehtmltags($value);
elseif ($typeofdata == 'day' || $typeofdata == 'datepicker') $ret .= dol_print_date($value, 'day');
elseif ($typeofdata == 'dayhour' || $typeofdata == 'datehourpicker') $ret .= dol_print_date($value, 'dayhour');
elseif (preg_match('/^select;/', $typeofdata))

View File

@ -664,8 +664,7 @@ function checkVal($out = '', $check = 'alphanohtml', $filter = null, $options =
}
break;
case 'restricthtml': // Recommended for most html textarea
$out = dol_string_onlythesehtmltags($out, 0);
// TODO We can also remove all javascripts reference
$out = dol_string_onlythesehtmltags($out, 0, 1, 1);
break;
case 'custom':
if (empty($filter)) return 'BadFourthParameterForGETPOST';
@ -1001,17 +1000,25 @@ function dol_string_nospecial($str, $newstr = '_', $badcharstoreplace = '')
/**
* Clean a string from all non printable ascii chars (0x00-0x1F and 0x7F). It removes also CR-LF
* Clean a string from all non printable ASCII chars (0x00-0x1F and 0x7F). It can also removes also Tab-CR-LF. UTF8 chars remains.
* This can be used to sanitize a string and view its real content. Some hacks try to obfuscate attacks by inserting non printable chars.
*
* Note, for information: UTF8 on 1 byte are: \x00-\7F
* 2 bytes are: byte 1 \xc0-\xdf, byte 2 = \x80-\xbf
* 3 bytes are: byte 1 \xe0-\xef, byte 2 = \x80-\xbf, byte 3 = \x80-\xbf
* 4 bytes are: byte 1 \xf0-\xf7, byte 2 = \x80-\xbf, byte 3 = \x80-\xbf, byte 4 = \x80-\xbf
* @param string $str String to clean
* @param int $removetabcrlf Remove also CR-LF
* @return string Cleaned string
*
* @see dol_sanitizeFilename(), dol_string_unaccent(), dol_string_nospecial()
*/
function dol_string_nounprintableascii($str)
function dol_string_nounprintableascii($str, $removetabcrlf = 1)
{
return preg_replace('/[\x00-\x1F\x7F]/u', '', $str);
if ($removetabcrlf) {
return preg_replace('/[\x00-\x1F\x7F]/u', '', $str); // /u operator makes UTF8 valid characters being ignored so are not included into the replace
} else {
return preg_replace('/[\x00-\x08\x11-\x12\x14-\x1F\x7F]/u', '', $str); // /u operator should make UTF8 valid characters being ignored so are not included into the replace
}
}
@ -5617,7 +5624,7 @@ function dol_string_nohtmltag($stringtoclean, $removelinefeed = 1, $pagecodeto =
$temp = preg_replace('/<br[^>]*>/i', "\n", $stringtoclean);
// We remove entities BEFORE stripping (in case of a separator char is encoded and not the other, the strip will fails)
$temp = dol_html_entity_decode($temp, ENT_COMPAT, $pagecodeto);
$temp = dol_html_entity_decode($temp, ENT_COMPAT|ENT_HTML5, $pagecodeto);
if ($strip_tags) {
$temp = strip_tags($temp);
@ -5652,7 +5659,7 @@ function dol_string_nohtmltag($stringtoclean, $removelinefeed = 1, $pagecodeto =
*
* @see dol_escape_htmltag() strip_tags() dol_string_nohtmltag() dol_string_neverthesehtmltags()
*/
function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1, $removeclassattribute = 1)
function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1, $removeclassattribute = 1, $cleanalsojavascript = 0)
{
$allowed_tags = array(
"html", "head", "meta", "body", "article", "a", "abbr", "b", "blockquote", "br", "cite", "div", "dl", "dd", "dt", "em", "font", "img", "ins", "hr", "i", "li", "link",
@ -5662,22 +5669,29 @@ function dol_string_onlythesehtmltags($stringtoclean, $cleanalsosomestyles = 1,
$allowed_tags_string = join("><", $allowed_tags);
$allowed_tags_string = '<'.$allowed_tags_string.'>';
if ($cleanalsosomestyles) {
$stringtoclean = preg_replace('/position\s*:\s*(absolute|fixed)\s*!\s*important/i', '', $stringtoclean); // Note: If hacker try to introduce css comment into string to bypass this regex, the string must also be encoded by the dol_htmlentitiesbr during output so it become harmless
}
if ($removeclassattribute) {
$stringtoclean = preg_replace('/(<[^>]+)\s+class=((["\']).*?\\3|\\w*)/i', '\\1', $stringtoclean);
}
// TODO Remove '/href=("|\'|)javascript/' string ?
$stringtoclean = dol_string_nounprintableascii($stringtoclean, 0);
$stringtoclean = preg_replace('/&colon;/i', ':', $stringtoclean);
$temp = strip_tags($stringtoclean, $allowed_tags_string);
if ($cleanalsosomestyles) { // Clean for remaining html tags
$stringtoclean = preg_replace('/position\s*:\s*(absolute|fixed)\s*!\s*important/i', '', $temp); // Note: If hacker try to introduce css comment into string to bypass this regex, the string must also be encoded by the dol_htmlentitiesbr during output so it become harmless
}
if ($removeclassattribute) { // Clean for remaining html tags
$stringtoclean = preg_replace('/(<[^>]+)\s+class=((["\']).*?\\3|\\w*)/i', '\\1', $temp);
}
// Remove 'javascript:' that we should not find into a text with
if ($cleanalsojavascript) {
$temp = preg_replace('/javascript\s*:/i', '', $temp);
}
return $temp;
}
/**
* Clean a string from some undesirable HTML tags.
* Note. Not enough secured as dol_string_onlythesehtmltags().
* Note. Not as secured as dol_string_onlythesehtmltags().
*
* @param string $stringtoclean String to clean
* @param array $disallowed_tags Array of tags not allowed
@ -5821,7 +5835,7 @@ function dol_htmlentitiesbr($stringtoencode, $nl2brmode = 0, $pagecodefrom = 'UT
*/
function dol_htmlentitiesbr_decode($stringtodecode, $pagecodeto = 'UTF-8')
{
$ret = dol_html_entity_decode($stringtodecode, ENT_COMPAT, $pagecodeto);
$ret = dol_html_entity_decode($stringtodecode, ENT_COMPAT|ENT_HTML5, $pagecodeto);
$ret = preg_replace('/'."\r\n".'<br(\s[\sa-zA-Z_="]*)?\/?>/i', "<br>", $ret);
$ret = preg_replace('/<br(\s[\sa-zA-Z_="]*)?\/?>'."\r\n".'/i', "\r\n", $ret);
$ret = preg_replace('/<br(\s[\sa-zA-Z_="]*)?\/?>'."\n".'/i', "\n", $ret);
@ -5845,7 +5859,7 @@ function dol_htmlcleanlastbr($stringtodecode)
* Replace html_entity_decode functions to manage errors
*
* @param string $a Operand a
* @param string $b Operand b (ENT_QUOTES=convert simple and double quotes)
* @param string $b Operand b (ENT_QUOTES|ENT_HTML5=convert simple, double quotes, colon, e accent, ...)
* @param string $c Operand c
* @param string $keepsomeentities Entities but &, <, >, " are not converted.
* @return string String decoded

View File

@ -66,7 +66,7 @@ function jsUnEscape($source)
$pos++;
}
}
return dol_html_entity_decode($decodedStr, ENT_COMPAT);
return dol_html_entity_decode($decodedStr, ENT_COMPAT|ENT_HTML5);
}

View File

@ -1260,7 +1260,7 @@ function pdf_getlinedesc($object, $i, $outputlangs, $hideref = 0, $hidedesc = 0,
// Manage HTML entities description test because $prodser->description is store with htmlentities but $desc no
$textwasmodified = false;
if (!empty($desc) && dol_textishtml($desc) && !empty($prodser->description) && dol_textishtml($prodser->description)) {
$textwasmodified = (strpos(dol_html_entity_decode($desc, ENT_QUOTES | ENT_HTML401), dol_html_entity_decode($prodser->description, ENT_QUOTES | ENT_HTML401)) !== false);
$textwasmodified = (strpos(dol_html_entity_decode($desc, ENT_QUOTES|ENT_HTML5), dol_html_entity_decode($prodser->description, ENT_QUOTES|ENT_HTML5)) !== false);
} else {
$textwasmodified = ($desc == $prodser->description);
}

View File

@ -142,7 +142,7 @@ class Odf
}
$this->vars[$tag] = $this->convertVarToOdf($value, $encode, $charset);
return $this;
}
@ -171,9 +171,9 @@ class Odf
'<style:style style:name="subText" style:family="text"><style:text-properties style:text-position="sub 58%" /></style:style>',
'<style:style style:name="supText" style:family="text"><style:text-properties style:text-position="super 58%" /></style:style>'
);
$convertedValue = $this->_replaceHtmlWithOdtTag($this->_getDataFromHtml($value), $customStyles, $fontDeclarations);
foreach ($customStyles as $key => $val) {
array_push($automaticStyles, '<style:style style:name="customStyle' . $key . '" style:family="text">' . $val . '</style:style>');
}
@ -400,7 +400,7 @@ class Odf
public function htmlToUTFAndPreOdf($value)
{
// We decode into utf8, entities
$value=dol_html_entity_decode($value, ENT_QUOTES);
$value=dol_html_entity_decode($value, ENT_QUOTES|ENT_HTML5);
// We convert html tags
$ishtml=dol_textishtml($value);

View File

@ -58,12 +58,16 @@ if (!empty($_SERVER['MAIN_SHOW_TUNING_INFO']))
*/
function testSqlAndScriptInject($val, $type)
{
$val = html_entity_decode($val, ENT_QUOTES); // So <svg o&#110;load='console.log(&quot;123&quot;)' become <svg onload='console.log(&quot;123&quot;)'
// Decode string first
// So <svg o&#110;load='console.log(&quot;123&quot;)' become <svg onload='console.log(&quot;123&quot;)'
// So "&colon;&apos;" become ":'" (due to ENT_HTML5)
$val = html_entity_decode($val, ENT_QUOTES|ENT_HTML5);
// TODO loop to decode until no more thing to decode ?
// We clean string because some hacks try to obfuscate evil strings by inserting non printable chars. Example: 'java(ascci09)scr(ascii00)ipt' is processed like 'javascript' (whatever is place of evil ascii char)
$val = preg_replace('/[\x00-\x1F\x7F]/u', '', $val); // We should use dol_string_nounprintableascii but function is not yet loaded/available
//var_dump($val);
// We should use dol_string_nounprintableascii but function is not yet loaded/available
$val = preg_replace('/[\x00-\x1F\x7F]/u', '', $val); // /u operator makes UTF8 valid characters being ignored so are not included into the replace
$inj = 0;
// For SQL Injection (only GET are used to be included into bad escaped SQL requests)

View File

@ -1471,7 +1471,7 @@ class Project extends CommonObject
$clone_project->note_public = '';
} else {
$this->db->begin();
$res = $clone_project->update_note(dol_html_entity_decode($clone_project->note_public, ENT_QUOTES), '_public');
$res = $clone_project->update_note(dol_html_entity_decode($clone_project->note_public, ENT_QUOTES|ENT_HTML5), '_public');
if ($res < 0)
{
$this->error .= $clone_project->error;
@ -1482,7 +1482,7 @@ class Project extends CommonObject
}
$this->db->begin();
$res = $clone_project->update_note(dol_html_entity_decode($clone_project->note_private, ENT_QUOTES), '_private');
$res = $clone_project->update_note(dol_html_entity_decode($clone_project->note_private, ENT_QUOTES|ENT_HTML5), '_private');
if ($res < 0)
{
$this->error .= $clone_project->error;

View File

@ -1675,7 +1675,7 @@ class Task extends CommonObject
$clone_task->note_public = '';
} else {
$this->db->begin();
$res = $clone_task->update_note(dol_html_entity_decode($clone_task->note_public, ENT_QUOTES), '_public');
$res = $clone_task->update_note(dol_html_entity_decode($clone_task->note_public, ENT_QUOTES|ENT_HTML5), '_public');
if ($res < 0)
{
$this->error .= $clone_task->error;
@ -1686,7 +1686,7 @@ class Task extends CommonObject
}
$this->db->begin();
$res = $clone_task->update_note(dol_html_entity_decode($clone_task->note_private, ENT_QUOTES), '_private');
$res = $clone_task->update_note(dol_html_entity_decode($clone_task->note_private, ENT_QUOTES|ENT_HTML5), '_private');
if ($res < 0)
{
$this->error .= $clone_task->error;

View File

@ -114,7 +114,7 @@ if ($conf->use_javascript_ajax)
$listofstatus = array(0, 1, 3, 9);
foreach ($listofstatus as $status)
{
$dataseries[] = array(dol_html_entity_decode($staticrecruitmentjobposition->LibStatut($status, 1), ENT_QUOTES), (isset($vals[$status]) ? (int) $vals[$status] : 0));
$dataseries[] = array(dol_html_entity_decode($staticrecruitmentjobposition->LibStatut($status, 1), ENT_QUOTES|ENT_HTML5), (isset($vals[$status]) ? (int) $vals[$status] : 0));
if ($status == RecruitmentJobPosition::STATUS_DRAFT) $colorseries[$status] = '-'.$badgeStatus0;
if ($status == RecruitmentJobPosition::STATUS_VALIDATED) $colorseries[$status] = $badgeStatus4;
if ($status == RecruitmentJobPosition::STATUS_RECRUITED) $colorseries[$status] = $badgeStatus6;
@ -190,7 +190,7 @@ if ($conf->use_javascript_ajax)
$listofstatus = array(0, 1, 3, 5, 8, 9);
foreach ($listofstatus as $status)
{
$dataseries[] = array(dol_html_entity_decode($staticrecruitmentcandidature->LibStatut($status, 1), ENT_QUOTES), (isset($vals[$status]) ? (int) $vals[$status] : 0));
$dataseries[] = array(dol_html_entity_decode($staticrecruitmentcandidature->LibStatut($status, 1), ENT_QUOTES|ENT_HTML5), (isset($vals[$status]) ? (int) $vals[$status] : 0));
if ($status == RecruitmentCandidature::STATUS_DRAFT) $colorseries[$status] = '-'.$badgeStatus0;
if ($status == RecruitmentCandidature::STATUS_VALIDATED) $colorseries[$status] = $badgeStatus1;
if ($status == RecruitmentCandidature::STATUS_CONTRACT_PROPOSED) $colorseries[$status] = $badgeStatus4;

View File

@ -482,15 +482,18 @@ if (empty($reshook)) {
}
if ($action == 'setsubject' && empty($object->subject)) {
$mesg .= ($mesg ? '<br>' : '').$langs->trans("ErrorFieldRequired", $langs->transnoentities("Subject"));
$error++;
setEventMessages($langs->trans("ErrorFieldRequired", $langs->transnoentities("Subject")), null, 'errors');
}
if (!$mesg) {
if (!$error) {
if ($object->update($user) >= 0) {
header("Location: ".$_SERVER['PHP_SELF']."?track_id=".$object->track_id);
exit;
} else {
$error++;
setEventMessages($object->error, $object->errors, 'errors');
}
$mesg = $object->error;
}
}
}
@ -507,6 +510,9 @@ if (empty($reshook)) {
$url = 'card.php?action=view&track_id='.$object->track_id;
header("Location: ".$url);
exit();
} else {
$error++;
setEventMessages($object->error, $object->errors, 'errors');
}
}
}
@ -543,6 +549,9 @@ if (empty($reshook)) {
$log_action .= Diff::toString(Diff::compare(strip_tags($oldvalue_message), strip_tags($object->message)));
setEventMessages($langs->trans('TicketMessageSuccesfullyUpdated'), null, 'mesgs');
} else {
$error++;
setEventMessages($object->error, $object->errors, 'errors');
}
}
@ -560,6 +569,9 @@ if (empty($reshook)) {
$url = 'card.php?action=view&track_id='.$object->track_id;
header("Location: ".$url);
exit();
} else {
$error++;
setEventMessages($object->error, $object->errors, 'errors');
}
}
}
@ -597,6 +609,9 @@ if (empty($reshook)) {
$log_action = $langs->trans('TicketLogPropertyChanged', $oldvalue_label, $newvalue_label);
setEventMessages($langs->trans('TicketUpdated'), null, 'mesgs');
} else {
$error++;
setEventMessages($object->error, $object->errors, 'errors');
}
$action = 'view';
}

View File

@ -1487,7 +1487,7 @@ class Ticket extends CommonObject
$message .= $langs->transnoentities('TicketNotificationRecipient').' : '.$recipient."\n";
$message .= "\n";
$message .= '* '.$langs->transnoentities('TicketNotificationLogMessage').' *'."\n";
$message .= dol_html_entity_decode($log_message, ENT_QUOTES)."\n";
$message .= dol_html_entity_decode($log_message, ENT_QUOTES|ENT_HTML5)."\n";
if ($info_sendto['source'] == 'internal') {
$url_internal_ticket = dol_buildpath('/ticket/card.php', 2).'?track_id='.$this->track_id;

View File

@ -1757,22 +1757,28 @@ if ($action == 'create' || $action == 'adduserldap')
{
print '<tr><td>'.$langs->trans("LinkToCompanyContact").'</td>';
print '<td>';
$s = '';
if (isset($object->socid) && $object->socid > 0)
{
$societe = new Societe($db);
$societe->fetch($object->socid);
print $societe->getNomUrl(1, '');
if ($societe->id > 0){
$s .= $societe->getNomUrl(1, '');
}
} else {
print $langs->trans("ThisUserIsNot");
$s .= $langs->trans("ThisUserIsNot");
}
if (!empty($object->contact_id))
{
$contact = new Contact($db);
$contact->fetch($object->contact_id);
if ($object->socid > 0) print ' / ';
else print '<br>';
print $contact->getNomUrl(1, '');
if ($contact->id > 0) {
if ($object->socid > 0 && $s) $s .= ' / ';
else $s .= '<br>';
$s .= $contact->getNomUrl(1, '');
}
}
print $s;
print '</td>';
print '</tr>'."\n";
}

View File

@ -64,7 +64,7 @@ if (empty($reshook)) {
if ($action == 'update' && $user->rights->user->user->creer && !$_POST["cancel"]) {
$db->begin();
$res = $object->update_note(dol_html_entity_decode(GETPOST('note_private', 'restricthtml'), ENT_QUOTES));
$res = $object->update_note(dol_html_entity_decode(GETPOST('note_private', 'restricthtml'), ENT_QUOTES|ENT_HTML5));
if ($res < 0) {
$mesg = '<div class="error">'.$adh->error.'</div>';
$db->rollback();

View File

@ -294,6 +294,46 @@ class SecurityTest extends PHPUnit\Framework\TestCase
return 0;
}
/**
* testDolStringOnlyTheseHtmlTags
*
* @return number
*/
public function testDolHTMLEntityDecode()
{
$stringtotest = 'a &colon; b &quot; c &#039; d &apos; e &eacute;';
$decodedstring = dol_html_entity_decode($stringtotest, ENT_QUOTES);
$this->assertEquals('a &colon; b " c \' d &apos; e é', $decodedstring, 'Function did not sanitize correclty');
$stringtotest = 'a &colon; b &quot; c &#039; d &apos; e &eacute;';
$decodedstring = dol_html_entity_decode($stringtotest, ENT_QUOTES|ENT_HTML5);
$this->assertEquals('a : b " c \' d \' e é', $decodedstring, 'Function did not sanitize correclty');
return 0;
}
/**
* testDolStringOnlyTheseHtmlTags
*
* @return number
*/
public function testDolStringOnlyTheseHtmlTags()
{
$stringtotest = '<a href="javascript:aaa">bbbڴ';
$decodedstring = dol_string_onlythesehtmltags($stringtotest, 1, 1, 1);
$this->assertEquals('<a href="aaa">bbbڴ', $decodedstring, 'Function did not sanitize correclty with test 1');
$stringtotest = '<a href="java'.chr(0).'script:aaa">bbbڴ';
$decodedstring = dol_string_onlythesehtmltags($stringtotest, 1, 1, 1);
$this->assertEquals('<a href="aaa">bbbڴ', $decodedstring, 'Function did not sanitize correclty with test 2');
$stringtotest = '<a href="javascript&colon;aaa">bbbڴ';
$decodedstring = dol_string_onlythesehtmltags($stringtotest, 1, 1, 1);
$this->assertEquals('<a href="aaa">bbbڴ', $decodedstring, 'Function did not sanitize correclty with test 3');
return 0;
}
/**
* testGetRandomPassword
*