Re: Eval use in XML_RPC
| From: | Justin Patrin | Date: | Thu, 28 Jul 2005 20:15:15 +0000 |
| Subject: | Re: Eval use in XML_RPC | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-39008@lists.php.net to get a copy of this message | ||
On 7/28/05, Joshua Eichorn <josh@bluga.net> wrote:
> After the last security problems in XML_RPC im surprised to see that no
> one has went and removed the use the eval from the code.
>
> From my quick audit I see no area where it actual needs to be used, and
> just from a security standpoint I don't think eval should be allowed in
> Any PEAR code.
I wouldn't go quite that far but it definately should not be used
without a very *very* good reason. That reason should IMHO involve
"impossible to do without eval" in some way.
>
> For example line 1629 of RPC.php is:
> @eval('$b->'.$id.' = $cont;');
And this is definately not a correct use. True, I used similar things
when I started in PHP because I didn't know about PHP's indirection
possibilities. This should be changed ASAP.
>
> This can easily be replaced by
> $b->$id = $cont;
>
> eval is now removed so things run faster, plus there is no possible
> exploit from a poorly escaped value in $id
>
> The other used of eval is at line 1331
>
> @eval('$v=' . $XML_RPC_xh[$parser]['st'] . '; $allOK=1;');
This one looks very straightforward but I'm sure I'm missing something
by not reviewing the rest of the code. This should also be replaced,
though, IMHO, as it doesn't look like something that is impossible
without eval.
>
> This seems to be used mainly to handle complex data types like arrays,
> though even strings and objects are pushed through it. Removing this
> will take reworking the XML_RPC_se function so its not a simple one
> liner, but it shouldn't be that hard to redo if you know how the process
> actually works.
>
--
Justin Patrin