Warning about eval

Kyle
(@keadie)

6 years, 6 months ago

I don’t know a lot about this plugin as I’m the server administrator/programmer, but I had to help my co-worker/web developer with it. I block eval on our client servers (a lot of shared hosts do) because of the security risk. On line 774 and 776 of wpgform-core.php, this plugin uses eval where there is no need to whatsoever. Unless the plugin author knows of a case I don’t see? Even still, as the creator of PHP stated:

“If eval() is the answer, you’re almost certainly asking the wrong question.”

Line: 774
Change
$x = eval(‘return sprintf(“%s%s%s”, $a, $op1, $b);’) ;
To
$x = sprintf(“%s%s%s”, $a, $op1, $b);

Line: 776
Change
$x = eval(‘return sprintf(“%s%s%s%s%s”, $a, $op1, $b, $op2, $c);’) ;
To
$x = sprintf(“%s%s%s%s%s”, $a, $op1, $b, $op2, $c);

Viewing 3 replies - 1 through 3 (of 3 total)

Plugin Author Mike Walsh
(@mpwalsh8)

6 years, 6 months ago

If the eval is removed then the CAPTCHA solution won’t ever be evaluated correctly. THe value of $x is the result of the CAPTCHA equation after it is evaluated. If $a = 2 and $b = 3 then $x should be 5 but if you take the eval out, $x will be the string “2 + 3” which won’t satisfy the CAPTCHA. Maybe there is a different way to do it but simply removing the eval will render the CAPTCHA broken.
victorbargains
(@victorbargains)

6 years, 2 months ago
Hi. I just started using this plugin and I too am concerned about the use of eval() since it poses a security risk.

Maybe I can help. I believe replacing lines 773-776 with this code will do the same as the eval, but in a type-safe way:
```
    function calculate2( $a, $op, $b ) {
        switch( $op ){
            case '+': return $a + $b;
            case '-': return $a - $b;
            case '*': return $a * $b;
        }
        return null;
    }

    if ((int)$wpgform_options['captcha_terms'] === 2)
        $x = calculate2( $a, $op1, $b );
    else if( $op1 == '*' || $op2 == '-' )
        $x = calculate2( calculate2( $a, $op1, $b ), $op2, $c );
    else
        $x = calculate2( $a, $op1, calculate2( $b, $op2, $c ) );
```
This code tests to see if the first operator is * or the second is -, because that will determine the order of operations, whether the answer is ($a $op1 $b) $op2 $c as opposed to $a $op1 ($b $op2 $c). I used this table to figure out what the logic should be, and I think it checks out.
```
//  a * b + c   // *+ should be (a * b) + c
//  a * b - c   // +- should be (a * b) - c    
//  a + b - c   // +- should be (a + b) - c
//  a - b - c   // -- should be (a - b) - c
//  a * b * c   // ** could be either (a * b) * c or a * (b * c)
//  a + b * c   // +* should be a + (b * c)
//  a - b * c   // -* should be a - (b * c)
//  a - b + c   // -+ should be a - (b + c)
//  a + b + c   // ++ could be either (a + b) + c or a + (b + c)
```
- This reply was modified 6 years, 2 months ago by victorbargains.
- This reply was modified 6 years, 2 months ago by victorbargains. Reason: fixing code formatting
Plugin Author Mike Walsh
(@mpwalsh8)

6 years, 2 months ago

I have finally had a chance to incorporate this suggestion in the plugin. It could use some testing before I push it out. I have posted it on my web site.

Viewing 3 replies - 1 through 3 (of 3 total)

The topic ‘Warning about eval’ is closed to new replies.