Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unchecked input in HTMLPurifier_UnitConverter round() function #386

Open
netdreamer opened this issue Oct 27, 2023 · 3 comments
Open

Unchecked input in HTMLPurifier_UnitConverter round() function #386

netdreamer opened this issue Oct 27, 2023 · 3 comments

Comments

@netdreamer
Copy link

Hello, I found an issue in the variable checking in round() function of the HTMLPurifier_UnitConverter.

If, for some reason, round() is called with an invalid value, it crashes.
There is no check that the passed value $n is really a number, before trying to do abs($n).

    private function round($n, $sigfigs)
    {
               $new_log = (int)floor(log(abs($n), 10)); // Number of digits left of decimal - 1
    ...

At the moment, I temporary fixed my issue by patching the function with a check:

    private function round($n, $sigfigs)
    {
        if (!is_numeric($n)) { return $n; }
        $new_log = (int)floor(log(abs($n), 10)); // Number of digits left of decimal - 1
    ...

This is the stack trace of the call that generated the issue:

PHP Fatal error:  Uncaught TypeError: abs(): Argument #1 ($num) must be of type int|float, string given in /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php:264
Stack trace:
#0 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php(264): abs()
#1 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/UnitConverter.php(178): HTMLPurifier_UnitConverter->round()
#2 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/Length.php(153): HTMLPurifier_UnitConverter->convert()
#3 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/AttrDef/CSS/Length.php(65): HTMLPurifier_Length->compareTo()
#4 /web/admin/vendor/ezyang/htmlpurifier/library/HTMLPurifier/AttrDef/CSS/Composite.php(39): HTMLPurifier_AttrDef_CSS_Length->validate()
@ezyang
Copy link
Owner

ezyang commented Oct 27, 2023

drat! send a PR?

@netdreamer
Copy link
Author

I'm preparing it, but It's a bit wider than expected: I found another similar issue (Length.php line 119).
$log = (int)floor(log(abs($n), 10));

$n is by definition a STRING, because it's the return value of $length->getN(), that returns a string...

So, every time you use it with as a parameter of an arithmetic function, it must be checked and/or converted to a number before calling abs() or similar functions.

@netdreamer
Copy link
Author

Sorry, I'm not very used to fixing code... I found that issue was already fixed in master: 43f49ac
But there are no releases with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants