SECURITY: Unsafe unserialization when working with schedules#35
SECURITY: Unsafe unserialization when working with schedules#35TheWitness merged 4 commits intodevelopfrom
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.
setup.php
Outdated
| $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).
No description provided.