Saving a skin doesn't look to sanitize the values before saving to a serialized object

Bug #1967000 reported by Robert Lyon
256
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
High
Robert Lyon
20.10
Fix Released
High
Unassigned
21.04
Fix Released
High
Unassigned
21.10
Fix Released
High
Unassigned
22.04
Fix Released
High
Robert Lyon

Bug Description

The $values when being saved to a skin either via designskinform_submit() function in skin/design.php or in importskinform_submit() function in skin/import.php do not look like they sanitize input before being saved as a serialized object.

Also there is a bug which is causing skins not to re-save after an upgrade
by default when you create a skin object it does an array merge with the defaults
but if you pull the viewskin out directly and un-serialize (like in style/skin.php)
it doesn't do the array merge with the defaults and a bunch of the values are unset
which causes a crash when it tries to do operations on null.

However, when you save it again, manually, it manages to update the values to include the new defaults so that bit works.

Need to tidy up these issues

Robert Lyon (robertl-9)
Changed in mahara:
milestone: none → 22.04.0
Revision history for this message
Robert Lyon (robertl-9) wrote :
Changed in mahara:
status: New → In Progress
Doris Tam (doristam)
Changed in mahara:
assignee: nobody → Robert Lyon (robertl-9)
Gold (gold.catalyst)
Changed in mahara:
status: In Progress → Fix Committed
Gold (gold.catalyst)
information type: Private Security → Public Security
Revision history for this message
Mahara Bot (dev-mahara) wrote : A change has been merged

Reviewed: https://reviews.mahara.org/c/mahara/+/12690
Committed: https://git.mahara.org/mahara/mahara/commit/ef1c8a7c5579d27dc852cc9c65b3e0819fe9c48a
Submitter: "Robert Lyon <email address hidden>"
Branch: 21.04_DEV

commit ef1c8a7c5579d27dc852cc9c65b3e0819fe9c48a
Author: Fergus Whyte <email address hidden>
Date: Wed Mar 30 09:37:12 2022 +1300

Bug 1967000: Make skin use the processed viewskin value provided by object, rather than pull the value out of the database directly.

This fixes issues where new defaults have not been added to skins yet.

Also some security checking around the viewskin serialized data to
make sore there is no object structure hiding in it that could cause
code execution

And fixing an issue if we pass an empty string rather than a hex
to fetch the rgb value - can happen on import

Change-Id: I6cb81bf9fa77d259d305cefecca1aa11e1a3b629
(cherry picked from commit d409628b7f72e224b865823d5254f511bd67d070)

To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.