security: replace eval() in RPN unary math functions with safe dispatch (GHSA-4mmp-mv2x-m9f6) - #808
security: replace eval() in RPN unary math functions with safe dispatch (GHSA-4mmp-mv2x-m9f6)#808bmfmancini wants to merge 3 commits into
Conversation
somethingwithproof
left a comment
somethingwithproof
left a comment
There was a problem hiding this comment.
Please add focused Pest coverage through Cacti Composer before merging. The PR description says unknown operators throw, but thold_rpn_math_unary() actually returns 0, silently converting a programming error into a valid threshold value. Make the invalid/default path fail closed (or prove it is unreachable and remove it), test every supported unary operator plus the invalid path, and coordinate/rebase with #773, which already replaces the eval dispatch in the same block. Current changed-line and integration checks are red.
…ch (S1523) Replace the eval()-based unary math function evaluation at line 403 with a new thold_rpn_math_unary() helper function that uses a switch statement to dispatch SIN, COS, TAN, ATAN, SQRT, FLOOR, CEIL, DEG2RAD, RAD2DEG, ABS, EXP, LOG to their native PHP equivalents. The operator was already whitelisted via in_array(, , true) and the operand validated with is_numeric(), but removing eval() eliminates the code injection risk entirely (CWE-95). Addresses SonarQube finding: php:S1523 at thold_functions.php:403
9ec0a97 to
f093027
Compare
|
The focused operator coverage and green CI are useful progress. This still overlaps the same evaluator block already changed by #773, and the invalid/default path returns numeric 0 after setting the error flag, which can look like a valid threshold value. Please rebase onto the selected evaluator implementation and make the invalid path fail closed. The identical workflow retry/matrix changes should be split into one dedicated PR rather than duplicated across the security series. |
Security Fix — GHSA-4mmp-mv2x-m9f6
Fixes GHSA-4mmp-mv2x-m9f6
Summary
The
thold_expression_math_rpn()function inthold_functions.phpused PHP'seval()to evaluate unary math functions (line 403) during RPN expression parsing for threshold expressions.Vulnerability Details
php:S1523(SonarQube)thold_functions.php, line 403Changes
Replaced
eval()with a safe dispatch functionthold_rpn_math_unary()that uses amatchexpression to call the appropriate PHP math function (SIN,COS,TAN,ATAN,SQRT,FLOOR,CEIL,DEG2RAD,RAD2DEG,ABS,EXP,LOG) without dynamic code execution.Testing
php -l thold_functions.php)SIN,COS,TAN,ATAN,SQRT,FLOOR,CEIL,DEG2RAD,RAD2DEG,ABS,EXP,LOG