Security and syntax issues

[otatarintseva]

Hello, we wanted to use this plugin, but found some security and syntax issues during the code review. The issues are listed below:

• directly calls to $_POST in several places,
• use unserialize on required_param of PARAM_RAW type - mod/tracker/views/profile.controller.php line 52:
  "Do not pass untrusted user input to unserialize() regardless of the options value of allowed_classes. Unserialization can result in code being loaded and executed due to object instantiation and autoloading, and a malicious user may be able to exploit this. Use a safe, standard data interchange format such as JSON (via json_decode() and json_encode()) if you need to pass serialized data to the user." (http://php.net/manual/en/function.unserialize.php)
• mod/tracker/rpclib.php function tracker_rpc_check () - uses $response->status = ... the variable is not initialized.
• not initialized variables - mod/tracker/locallib.php
  • $descriptiontoken - line 454
  • $sql - line 536
  • $cookie - line 864
  • $context - line 1116
  • $ccuser - line1373
  • $CFG - line 1608
  • $ticketurl - line 1930
  • $stats - line 2111 - will work, but it is goot style and more safely to initialize the variable
  • $sr - typo, should be $str - could cause incorrect html code generation.
• Similar things found in other files.
• mod/tracker/views/editwatch.html - html syntax error in line 40

[vfremaux]

Thanks to report those attention points. We are going to have a refactor work soon on these inputs. Actually tracker has somewhere very old coding parts that were not reviewed.... Cheers Valery. Le mer. 24 oct. 2018 à 16:50, otatarintseva <[email protected]> a écrit :

…

Hello, we wanted to use this plugin, but found some security and syntax issues during the code review. The issues are listed below: - directly calls to $_POST in several places, - use unserialize on required_param of PARAM_RAW type - mod/tracker/views/profile.controller.php line 52: "Do not pass untrusted user input to unserialize() regardless of the options value of allowed_classes. Unserialization can result in code being loaded and executed due to object instantiation and autoloading, and a malicious user may be able to exploit this. Use a safe, standard data interchange format such as JSON (via json_decode() and json_encode()) if you need to pass serialized data to the user." ( http://php.net/manual/en/function.unserialize.php) - mod/tracker/rpclib.php function tracker_rpc_check () - uses $response->status = ... the variable is not initialized. - not initialized variables - mod/tracker/locallib.php - $descriptiontoken - line 454 - $sql - line 536 - $cookie - line 864 - $context - line 1116 - $ccuser - line1373 - $CFG - line 1608 - $ticketurl - line 1930 - $stats - line 2111 - will work, but it is goot style and more safely to initialize the variable - $sr - typo, should be $str - could cause incorrect html code generation. - Similar things found in other files. - mod/tracker/views/editwatch.html - html syntax error in line 40 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#23>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABrId0JlSw-wO1acgAH7t_X09jpQKGhiks5uoH46gaJpZM4X4Gwr> .