Scan uploaded levels for ACE #1002
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * In the future use this class for level validation. Data or other parameters. | ||
| */ | ||
| class LevelParser { | ||
| public $data = []; | ||
| const MAX_GROUPS_21 = 1099; | ||
| const MAX_GROUPS_22 = 9999; | ||
|
|
||
| /** | ||
| * Convert GD's weird "keyed" array strings to dictionary. Works with out-of-order keys. | ||
| * | ||
| * @param $list string | ||
| * @param $separator string | ||
| * @return array | ||
| */ | ||
| public static function map($list, $separator) { | ||
| $bits = explode($separator, $list); | ||
| $array = []; | ||
| for ($i = 1; $i < count($bits); $i += 2) { | ||
| $array[$bits[$i - 1]] = $bits[$i]; | ||
| } | ||
| return $array; | ||
| } | ||
|
|
||
| /** | ||
| * Flatten dictionary to single delimited string | ||
| * | ||
| * @param $dict array|object | ||
| * @param $separator string | ||
| * @return string | ||
| */ | ||
| public static function unmap($dict, $separator) { | ||
| $string = ''; | ||
| foreach ($dict as $key => $value) { | ||
| $string[] .= "${separator}${key}${separator}${value}"; | ||
| } | ||
| return $string; | ||
| } | ||
|
|
||
| /** | ||
| * @param $string string | ||
| * @return false|string | ||
| */ | ||
| public static function base64_urlencode($string) { | ||
| return base64_encode(strtr($string, '+/', '-_')); | ||
| } | ||
|
|
||
| /** | ||
| * @param $string string | ||
| * @return false|string | ||
| */ | ||
| public static function base64_urldecode($string) { | ||
| return base64_decode(strtr($string, '-_', '+/'), true); | ||
| } | ||
|
|
||
| /** | ||
| * Validates level data for various things (currently only ACE exploit) | ||
| * | ||
| * @return bool | ||
| */ | ||
| public static function validate($levelString, $version) { | ||
| include '../../config/security.php'; | ||
| try { | ||
| $data = $levelString; | ||
|
|
||
| // Decode (strict mode) if falsy then | ||
| $decoded = self::base64_urldecode($levelString); | ||
| if ($decoded !== false) $data = $decoded; | ||
|
|
||
| // Check for zlib magic | ||
| if(bin2hex(substr($data, 0, 3)) === '1f8b08') { | ||
| $data = zlib_decode($data, $maxUncompressedLevelSize); | ||
| if (!$data) { | ||
| return false; | ||
| } | ||
| } | ||
| // Check if result invalid (any character outside ascii range). Better heuristic for detecting junk? | ||
| if (preg_match('/[^\x20-\x7e]/', $data)) return false; | ||
| $objs = explode(';', $data); | ||
| // Skip level header | ||
| for ($i = 1; $i < count($objs); $i++) { | ||
| $obj = self::map($objs[$i], ','); | ||
| // Clamp groups based on version | ||
| if (array_key_exists(80, $obj)) { | ||
| $id = intval($obj[80]); | ||
| if ($id > ($version > 21 ? self::MAX_GROUPS_22 : self::MAX_GROUPS_21) || $id < 0) return false; | ||
|
Comment on lines
+81
to
+89
There was a problem hiding this comment. allowing all ascii characters with In the client, this data is parsed using
C Would be a simple fix if it wasn't for the fact that a larger exponent results in the method erroring to This problem allows you to use values in the client while also tricking this code into believing its safe There was a problem hiding this comment. Yes that's right. It's also possible to insert other invalid values while conforming to object format. This PR is primarily concerned with ACE protection not validating all level data (though this is worth looking into more). The check for non-ASCII characters is simply a heuristic for detecting malformed level data (such as attempting to upload arbitrary binary files or catching possible failure of the data decompression) and is not intended as a catch-all solution for level validation. |
||
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh i don't see the point of making this a single member class. this data member is basically useless anyways (unless you were planning on extending the functionality of this class?). just make validate static and pass the level string as its param
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes in the future functionality could be expanded on validating other parts of the level request. However for now I have implemented your suggestions