Forbid unserialize() method. #52
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #52 +/- ##
=========================================
Coverage 96.46% 96.46%
Complexity 517 517
=========================================
Files 22 22
Lines 1471 1471
=========================================
Hits 1419 1419
Misses 52 52 ☔ View full report in Codecov by Sentry. |
|
I like the idea, mainly because I don't like the idea of guaranteeing that the internal structure of the object won't change (i.e. private variables). However, it probably needs some kind of official approval and an update in the Moodle developer documentation. Found this relevant issue in the Moodle tracker: https://tracker.moodle.org/browse/MDLSITE-3242 |
|
Hi @jrchamp, absolutely - this'll go through the regular policy voting issues much like https://tracker.moodle.org/browse/CONTRIB-4146 After that point (if it's agreed) then we'll update https://docs.moodle.org/dev/Coding_style#Dangerous_functions_and_constructs and land this pull request |
Was it? MDLSITE-3242 |
|
Yup
|
|
Apologies - I misread this one. The MDSLITE is still open and undecided. |
Can lead to code execution exploits if not used properly with user supplied data. There are better methods of data exchange.
paulholden
force-pushed
the
forbid-unserialize
branch
from
3a33e5f
to
3a31c65
Compare
| @@ -65,5 +63,6 @@ class ForbiddenFunctionsSniff extends GenericForbiddenFunctionsSniff { | |||
| 'print_object' => null, | |||
| // Dangerous functions. From coding style. | |||
| 'extract' => null, | |||
| 'unserialize' => null, | |||
There was a problem hiding this comment.
While I agree that we should avoid serialize and unserialize entirely, it might be nice to let people know that unserialize_object() is the Moodle-provided alternative to unserialize.
| 'unserialize' => null, | |
| 'unserialize' => 'unserialize_object', |
Can lead to code execution exploits if not used properly with user supplied data. There are better methods of data exchange.
See also moodle/devdocs#822