Skip to content
Permalink
Browse files Browse the repository at this point in the history
Remove pre-processing of PHP code, disallow I/O streams as file input
  • Loading branch information
bsweeney committed Mar 11, 2014
1 parent 400dbd6 commit 23a6939
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
4 changes: 4 additions & 0 deletions dompdf.php
Expand Up @@ -227,6 +227,10 @@ function getoptions() {
}
}

if($file_parts['protocol'] === 'php://') {
throw new DOMPDF_Exception("Permission denied on $file. This script does not allow PHP streams.");
}

$outfile = "dompdf_out.pdf"; # Don't allow them to set the output file
$save_file = false; # Don't save the file

Expand Down
7 changes: 0 additions & 7 deletions include/dompdf.cls.php
Expand Up @@ -580,13 +580,6 @@ function load_html($str, $encoding = null) {
$str = substr($str, 3);
}

// Parse embedded php, first-pass
if ( $this->get_option("enable_php") ) {
ob_start();
eval("?" . ">$str");
$str = ob_get_clean();
}

// if the document contains non utf-8 with a utf-8 meta tag chars and was
// detected as utf-8 by mbstring, problems could happen.
// http://devzone.zend.com/article/8855
Expand Down

6 comments on commit 23a6939

@u01jmg3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the pre-processing of PHP code is causing me issues:

issue

Is there a workaround for my issue?

@bsweeney
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the pre-processing did was eval() the document/string prior to import into dompdf. We want to move away from that kind of functionality, which is secondary to the main purpose of dompdf, in order to limit the potential attack surface.

How are you using dompdf? Based on the call stack it looks like you are using the class. If you need to pre-process your code you can just add a step to eval() your code prior to passing it to dompdf. If that's not possible, you can add the removed code back in to dompdf.cls.php, it shouldn't affect anything otherwise.

@u01jmg3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bsweeney for your quick reply

I'd like to getaway from having that code if you've removed it from the master branch of dompdf

I'm a bit unsure how to eval() my code prior to passing it to dompdf. I use this at the bottom of my PHP file which I convert to a PDF using dompdf:

<script type="text/php">
    if (isset($pdf)) {
        $w = $pdf->get_width();
        $h = $pdf->get_height();

        $text = Font_Metrics::get_font('Lucida Sans');
        $footer = $pdf->open_object();
        $pdf->add_object($footer, 'all');
        $pdf->page_text(30, $h-30, strtoupper('<?php echo implode(" ", array($forename1{0} . "." . (strlen($surname) > 13 ? substr($surname, 0, 6) . "..." . substr($surname, -5, 5) : $surname), "(" . $personal_id . "-" . $application_id . ")")); ?>'), $font, 10, array(0, 0, 0));
    }
</script>

And I use this to run/call dompdf:

spl_autoload_register('DOMPDF_autoload');
$dompdf = new DOMPDF();
$dompdf->set_paper('a4', 'portrait');
$dompdf->load_html(file_get_contents($html));
$dompdf->render();

if($stream){
    $dompdf->stream($filename . '.pdf');
} else {
    $output = $dompdf->output();
    file_put_contents($dirname . $filename . '.pdf', $output);
}

@bsweeney
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline script will still run, but as you've seen your code that generates the inline script will have to be modified. Since you're not doing anything with page numbering in that code you could just as easily re-write it using a fixed-position element. replace your inline script with something like the following:

<div style="font-family: Lucia Sans, sans-serif; font-size: 10pt; position: fixed; bottom: 30px; left: 30px; right: 30px;">
  <?php echo strtoupper(implode(" ", array($forename1{0} . "." . (strlen($surname) > 13 ? substr($surname, 0, 6) . "..." . substr($surname, -5, 5) : $surname), "(" . $personal_id . "-" . $application_id . ")"))); ?>
</div>

If that doesn't work for you the easiest fix would be to run the string generation as part of the inline script. That would only work if the variables are available globally.

<script type="text/php">
    if (isset($pdf)) {
        $w = $pdf->get_width();
        $h = $pdf->get_height();

        $text = Font_Metrics::get_font('Lucida Sans');
        $footer = $pdf->open_object();
        $pdf->add_object($footer, 'all');
        $pdf->page_text(30, $h-30, strtoupper(implode(" ", array($GLOBALS['forename1']{0} . "." . (strlen($GLOBALS['surname']) > 13 ? substr(GLOBALS['$surname'], 0, 6) . "..." . substr($GLOBALS['surname'], -5, 5) : $GLOBALS['surname']), "(" . $GLOBALS['personal_id'] . "-" . $GLOBALS['application_id'] . ")"))), $font, 10, array(0, 0, 0));
    }
</script>

(You'll have to check that code ... I didn't).

@u01jmg3
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @bsweeney

I ended up just changing my call to be this:

spl_autoload_register('DOMPDF_autoload');
$dompdf = new DOMPDF();
$dompdf->set_paper('a4', 'portrait');

//start new code
ob_start();
include $html;
$html = ob_get_clean();
//end new code

$dompdf->load_html($html);
$dompdf->render();

if($stream){
    $dompdf->stream($filename . '.pdf');
} else {
    $output = $dompdf->output();
    file_put_contents($dirname . $filename . '.pdf', $output);
}

@bsweeney
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an easy way around the issue. Glad you were able to come to a quick resolution.

Please sign in to comment.