From d2279457f0cab31b7e27aada981f561693ba8a52 Mon Sep 17 00:00:00 2001 From: Paola Maneggia Date: Fri, 28 Oct 2022 11:49:15 +0200 Subject: [PATCH 1/2] MDL-76092 core_backup: Fix PHP8.0 vulnerability convert_params_to_values --- backup/util/dbops/backup_structure_dbops.class.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backup/util/dbops/backup_structure_dbops.class.php b/backup/util/dbops/backup_structure_dbops.class.php index 8ffc63e2efd5a..b133dd0548530 100644 --- a/backup/util/dbops/backup_structure_dbops.class.php +++ b/backup/util/dbops/backup_structure_dbops.class.php @@ -72,7 +72,7 @@ public static function convert_params_to_values($params, $processor) { throw new base_element_struct_exception('valueofparamelementnotset', $param->get_name()); } - } else if ($param < 0) { // Possibly one processor variable, let's process it + } else if (is_numeric($param) && $param < 0) { // Possibly one processor variable, let's process it // See @backup class for all the VAR_XXX variables available. // Note1: backup::VAR_PARENTID is handled by nested elements themselves // Note2: trying to use one non-existing var will throw exception From 956fd2bab569e14935a929b467694330b2ab0354 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 23 Aug 2023 13:25:41 +0800 Subject: [PATCH 2/2] MDL-76092 core_backup: Unit tests for convert_params_to_values --- .../dbops/backup_structure_dbops.class.php | 2 +- .../tests/backup_structure_dbops_test.php | 117 ++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 backup/util/dbops/tests/backup_structure_dbops_test.php diff --git a/backup/util/dbops/backup_structure_dbops.class.php b/backup/util/dbops/backup_structure_dbops.class.php index b133dd0548530..249a55d27da3b 100644 --- a/backup/util/dbops/backup_structure_dbops.class.php +++ b/backup/util/dbops/backup_structure_dbops.class.php @@ -72,7 +72,7 @@ public static function convert_params_to_values($params, $processor) { throw new base_element_struct_exception('valueofparamelementnotset', $param->get_name()); } - } else if (is_numeric($param) && $param < 0) { // Possibly one processor variable, let's process it + } else if (is_int($param) && $param < 0) { // Possibly one processor variable, let's process it // See @backup class for all the VAR_XXX variables available. // Note1: backup::VAR_PARENTID is handled by nested elements themselves // Note2: trying to use one non-existing var will throw exception diff --git a/backup/util/dbops/tests/backup_structure_dbops_test.php b/backup/util/dbops/tests/backup_structure_dbops_test.php new file mode 100644 index 0000000000000..128ccdac66937 --- /dev/null +++ b/backup/util/dbops/tests/backup_structure_dbops_test.php @@ -0,0 +1,117 @@ +. + +namespace core_backup; + +use backup_structure_dbops; + +/** + * Tests for backup_structure_dbops + * + * @package core_backup + * @category test + * @copyright 2023 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \backup_structure_dbops + */ +class backup_structure_dbops_test extends \advanced_testcase { + public static function setUpBeforeClass(): void { + global $CFG; + parent::setUpBeforeClass(); + require_once("{$CFG->dirroot}/backup/util/includes/backup_includes.php"); + } + + /** + * Tests for convert_params_to_values. + * + * @dataProvider convert_params_to_values_provider + * @param array $params + * @param mixed $processor + * @param array $expected + */ + public function test_convert_params_to_values( + array $params, + $processor, + array $expected, + ): void { + if (is_callable($processor)) { + $newprocessor = $this->createMock(\backup_structure_processor::class); + $newprocessor->method('get_var')->willReturnCallback($processor); + $processor = $newprocessor; + } + + $result = backup_structure_dbops::convert_params_to_values($params, $processor); + + $this->assertEqualsCanonicalizing($expected, $result); + } + + /** + * Data provider for convert_params_to_values_provider. + */ + public static function convert_params_to_values_provider(): array { + return [ + 'String value is not processed' => [ + ['/0/1/2/345'], + null, + ['/0/1/2/345'], + ], + 'Positive integer' => [ + [123, 456], + null, + [123, 456], + ], + 'Negative integer' => [ + [-42], + function () { + return 'Life, the Universe, and Everything'; + }, + ['Life, the Universe, and Everything'], + ], + 'Mix of strings, and ints with a processor' => [ + ['foo', 123, 'bar', -42], + function () { + return 'Life, the Universe, and Everything'; + }, + ['foo', 123, 'bar', 'Life, the Universe, and Everything'], + ], + ]; + } + + /** + * Tests for convert_params_to_values with an atom. + */ + public function test_convert_params_to_values_with_atom(): void { + $atom = $this->createMock(\base_atom::class); + $atom->method('is_set')->willReturn(true); + $atom->method('get_value')->willReturn('Some atomised value'); + + $result = backup_structure_dbops::convert_params_to_values([$atom], null); + + $this->assertEqualsCanonicalizing(['Some atomised value'], $result); + } + + /** + * Tests for convert_params_to_values with an atom without any value. + */ + public function test_convert_params_to_values_with_atom_no_value(): void { + $atom = $this->createMock(\base_atom::class); + $atom->method('is_set')->willReturn(false); + $atom->method('get_name')->willReturn('Atomisd name'); + + $this->expectException(\base_element_struct_exception::class); + backup_structure_dbops::convert_params_to_values([$atom], null); + } +}