composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-05-18 23:08:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

#970 and #650 Corrupted attachements. Reworked the cleanup of undesired output, to protect it against the case when the output buffer is unfortunately closed. On the other hand, I found out that several output buffer can be stacked. Thus the protection could be further improved (difficulty: that can be web server dependent).

Browse Source 
SVN:trunk[3376]
This commit is contained in:
Romain Quetiez
2014-10-23 15:48:49 +00:00
parent b65131e4f5
commit cd7490472e
5 changed files with 61 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
application/ajaxwebpage.class.inc.php
View File

@@ -1,5 +1,5 @@
<?php
// Copyright (C) 2010-2012 Combodo SARL
// Copyright (C) 2010-2014 Combodo SARL
//
// This file is part of iTop.
//
@@ -225,9 +225,8 @@ EOF
EOF
);
}
$s_captured_output = ob_get_contents();
ob_end_clean();
if (($this->sContentType == 'text/html') && ($this->sContentDisposition == 'inline'))
$s_captured_output = $this->ob_get_clean_safe();
if (($this->sContentType == 'text/html') && ($this->sContentDisposition == 'inline'))
{
// inline content != attachment && html => filter all scripts for malicious XSS scripts
echo self::FilterXSS($this->s_content);
@@ -393,4 +392,3 @@ EOF
}
}
?>

9
application/csvpage.class.inc.php
View File

@@ -1,5 +1,5 @@
<?php
// Copyright (C) 2010-2012 Combodo SARL
// Copyright (C) 2010-2014 Combodo SARL
//
// This file is part of iTop.
//
@@ -39,7 +39,11 @@ class CSVPage extends WebPage
public function output()
{
$this->add_header("Content-Length: ".strlen(trim($this->s_content)));
$this->add_header("Content-Length: ".strlen(trim($this->s_content)));
// Get the unexpected output but do nothing with it
$sTrash = $this->ob_get_clean_safe();
foreach($this->a_headers as $s_header)
{
header($s_header);
@@ -105,4 +109,3 @@ class CSVPage extends WebPage
}
}
?>

4
application/itopwebpage.class.inc.php
View File

@@ -587,8 +587,7 @@ EOF
header($s_header);
}
}
$s_captured_output = ob_get_contents();
ob_end_clean();
$s_captured_output = $this->ob_get_clean_safe();
$sHtml = "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n";
$sHtml .= "<html>\n";
$sHtml .= "<head>\n";
@@ -1039,4 +1038,3 @@ EOF
$this->m_sMessage = $sMessage;
}
}
?>

57
application/webpage.class.inc.php
View File

@@ -1,5 +1,5 @@
<?php
// Copyright (C) 2010-2013 Combodo SARL
// Copyright (C) 2010-2014 Combodo SARL
//
// This file is part of iTop.
//
@@ -68,6 +68,7 @@ class WebPage implements Page
protected $sContentType;
protected $sContentDisposition;
protected $sContentFileName;
protected $bTrashUnexpectedOutput;
protected $s_sOutputFormat;
protected $a_OutputOptions;
@@ -88,6 +89,7 @@ class WebPage implements Page
$this->sContentType = '';
$this->sContentDisposition = '';
$this->sContentFileName = '';
$this->bTrashUnexpectedOutput = false;
$this->s_OutputFormat = utils::ReadParam('output_format', 'html');
$this->a_OutputOptions = array();
ob_start(); // Start capturing the output
@@ -392,22 +394,55 @@ class WebPage implements Page
}
/**
* Discard unexpected output data
* Discard unexpected output data (such as PHP warnings)
* This is a MUST when the Page output is DATA (download of a document, download CSV export, download ...)
*/
public function TrashUnexpectedOutput()
{
// This protection is redundant with a protection implemented in MetaModel::IncludeModule
// which detects such issues while loading module files
// Here, the purpose is to detect and discard characters produced by the code execution (echo)
$sPreviousContent = ob_get_clean();
if (trim($sPreviousContent) != '')
$this->bTrashUnexpectedOutput = true;
}
/**
* Read the output buffer and deal with its contents:
* - trash unexpected output if the flag has been set
* - report unexpected behaviors such as the output buffering being stopped
*
* Possible improvement: I've noticed that several output buffers are stacked,
* if they are not empty, the output will be corrupted. The solution would
* consist in unstacking all of them (and concatenate the contents).
*/
protected function ob_get_clean_safe()
{
$sOutput = ob_get_contents();
if ($sOutput === false)
{
if (Utils::GetConfig() && Utils::GetConfig()->Get('debug_report_spurious_chars'))
$sMsg = "Design/integration issue: No output buffer. Some piece of code has called ob_get_clean() or ob_end_clean() without calling ob_start()";
if ($this->bTrashUnexpectedOutput)
{
IssueLog::Error("Output already started before downloading file:\nContent was:'$sPreviousContent'\n");
IssueLog::Error($sMsg);
$sOutput = '';
}
else
{
$sOutput = $sMsg;
}
}
else
{
ob_end_clean(); // on some versions of PHP doing so when the output buffering is stopped can cause a notice
if ($this->bTrashUnexpectedOutput)
{
if (trim($sOutput) != '')
{
if (Utils::GetConfig() && Utils::GetConfig()->Get('debug_report_spurious_chars'))
{
IssueLog::Error("Trashing unexpected output:'$s_captured_output'\n");
}
}
$sOutput = '';
}
}
return $sOutput;
}
/**
@@ -419,8 +454,8 @@ class WebPage implements Page
{
header($s_header);
}
$s_captured_output = ob_get_contents();
ob_end_clean();
$s_captured_output = $this->ob_get_clean_safe();
echo "<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Strict//EN\" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd\">\n";
echo "<html>\n";
echo "<head>\n";

17
application/xmlpage.class.inc.php
View File

@@ -1,5 +1,5 @@
<?php
// Copyright (C) 2010-2013 Combodo SARL
// Copyright (C) 2010-2014 Combodo SARL
//
// This file is part of iTop.
//
@@ -51,6 +51,9 @@ class XMLPage extends WebPage
{
if (!$this->m_bPassThrough)
{
// Get the unexpected output but do nothing with it
$sTrash = $this->ob_get_clean_safe();
$this->s_content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?".">\n".trim($this->s_content);
$this->add_header("Content-Length: ".strlen($this->s_content));
foreach($this->a_headers as $s_header)
@@ -79,8 +82,7 @@ class XMLPage extends WebPage
}
else
{
$s_captured_output = ob_get_contents();
ob_end_clean();
$s_captured_output = $this->ob_get_clean_safe();
foreach($this->a_headers as $s_header)
{
header($s_header);
@@ -101,13 +103,4 @@ class XMLPage extends WebPage
public function table($aConfig, $aData, $aParams = array())
{
}
public function TrashUnexpectedOutput()
{
if (!$this->m_bPassThrough)
{
parent::TrashUnexpectedOutput();
}
}
}
?>