SECURITY: Unsafe unserialization when working with schedules #35
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the maint plugin to address unsafe unserialization when processing selected schedule/device items, along with a version bump and changelog cleanup.
Changes:
- Replace direct
unserialize()usage withsanitize_unserialize_selected_items()forselected_itemshandling. - Add
CHANGELOG.mdand remove the embedded changelog fromREADME.md. - Bump plugin version to 1.3 and update copyright year.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| setup.php | Switches selected-items parsing to a sanitized unserialize helper in device actions (but currently contains a variable mismatch bug). |
| README.md | Removes old embedded changelog content and updates copyright year. |
| INFO | Bumps plugin version from 1.2 to 1.3. |
| CHANGELOG.md | Adds a dedicated changelog file for releases (contains a minor typo). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $selected_items = sanitize_unserialize_selected_items(get_nfilter_request_var('selected_items')); | ||
|
|
||
| if (is_array($selected)) { | ||
| foreach ($selected as $host_id) { | ||
| db_execute_prepared('REPLACE INTO plugin_maint_hosts (type, host, schedule) VALUES (1, ?, ?)', array((int)$host_id, (int)$schedule_id)); |
There was a problem hiding this comment.
$selected_items is assigned from sanitize_unserialize_selected_items(...), but the subsequent is_array()/foreach use $selected, which is undefined. This prevents any devices from being associated with the new schedule (and will raise a PHP notice). Use $selected_items consistently in the is_array check and foreach loop (or match maint.php’s pattern of checking $selected_items != false).
| $selected_items = sanitize_unserialize_selected_items(get_nfilter_request_var('selected_items')); | ||
| if (is_array($selected)) { | ||
| foreach ($selected as $host_id) { | ||
| db_execute_prepared('REPLACE INTO plugin_maint_hosts (type, host, schedule) VALUES (1, ?, ?)', array((int)$host_id, $schedule_id)); | ||
| $added++; |
There was a problem hiding this comment.
$selected_items is populated, but $selected is used in is_array()/foreach, so no devices will be added to the chosen schedule. Replace $selected with $selected_items here as well.
| * issue#15: Fix webseer tab to not show items before schedule is created | ||
| * feature#14: Webseer tab functional (webseer plugin update required to use schedule) | ||
| * feature#18: Device tab filter | ||
| * featuer#29: Add Servcheck |
There was a problem hiding this comment.
Typo in changelog entry: "featuer#29" should be "feature#29".
| * featuer#29: Add Servcheck | |
| * feature#29: Add Servcheck |
No description provided.