Skip to content

Commit c55432c

Browse files
dereuromarkclaude
dereuromark
and
claude
committed
Fix security vulnerabilities and add type hints
- Fix unsafe deserialization in ObjectType by whitelisting safe classes - Fix path traversal vulnerability in GoogleMapHelper icon method - Fix SQL injection in GeocoderBehavior distanceConditions - Implement isAccurateEnough() method in Geocoder - Add missing type hints to GoogleMapHelper methods - Fix type casting in GeoCoordinate::fromString() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 70ae8c1 commit c55432c

File tree

5 files changed

+144
-29
lines changed

5 files changed

+144
-29
lines changed

‎src/Database/Type/ObjectType.php‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,13 @@
44

55
use Cake\Database\Driver;
66
use Cake\Database\Type\BaseType;
7+
use Cake\I18n\Date;
8+
use Cake\I18n\DateTime;
9+
use DateInterval;
10+
use DateTimeImmutable;
11+
use DateTimeZone;
712
use PDO;
13+
use stdClass;
814

915
/**
1016
* This can serialize and unserialize objects.
@@ -22,7 +28,18 @@ public function toPHP(mixed $value, Driver $driver): mixed {
2228
return $value;
2329
}
2430

25-
return unserialize($value);
31+
// Allow only safe classes to prevent object injection attacks
32+
$allowedClasses = [
33+
DateTime::class,
34+
DateTimeImmutable::class,
35+
DateTimeZone::class,
36+
DateInterval::class,
37+
DateTime::class,
38+
Date::class,
39+
stdClass::class,
40+
];
41+
42+
return unserialize($value, ['allowed_classes' => $allowedClasses]);
2643
}
2744

2845
/**
@@ -38,7 +55,18 @@ public function marshal(mixed $value): mixed {
3855
return $value;
3956
}
4057

41-
return unserialize($value);
58+
// Allow only safe classes to prevent object injection attacks
59+
$allowedClasses = [
60+
DateTime::class,
61+
DateTimeImmutable::class,
62+
DateTimeZone::class,
63+
DateInterval::class,
64+
DateTime::class,
65+
Date::class,
66+
stdClass::class,
67+
];
68+
69+
return unserialize($value, ['allowed_classes' => $allowedClasses]);
4270
}
4371

4472
/**

‎src/Geocoder/GeoCoordinate.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public static function fromString(string $coordinate): static {
8888

8989
[$lat, $lng] = explode(',', $coordinate);
9090

91-
return new static($lat, $lng);
91+
return new static((float)$lat, (float)$lng);
9292
}
9393

9494
/**

‎src/Geocoder/Geocoder.php‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -336,14 +336,79 @@ public function containsAccurateEnough(AddressCollection $addresses) {
336336
* @return bool
337337
*/
338338
public function isAccurateEnough(Location $address) {
339-
$levels = array_keys($this->_types);
340-
$values = array_values($this->_types);
341-
$map = array_combine($levels, $values);
342-
343339
$expected = $this->_config['minAccuracy'];
340+
if (!$expected) {
341+
return true;
342+
}
343+
344+
$minAccuracyIndex = array_search($expected, $this->_types, true);
345+
if ($minAccuracyIndex === false) {
346+
return true;
347+
}
348+
349+
$adminLevels = $address->getAdminLevels();
350+
$adminLevelMap = [
351+
static::TYPE_AAL1 => 1,
352+
static::TYPE_AAL2 => 2,
353+
static::TYPE_AAL3 => 3,
354+
static::TYPE_AAL4 => 4,
355+
static::TYPE_AAL5 => 5,
356+
];
357+
358+
// Check all accuracy levels from minimum required to most accurate
359+
for ($i = $minAccuracyIndex; $i < count($this->_types); $i++) {
360+
$type = $this->_types[$i];
361+
switch ($type) {
362+
case static::TYPE_COUNTRY:
363+
if ($address->getCountry() !== null) {
364+
return true;
365+
}
366+
367+
break;
368+
case static::TYPE_AAL1:
369+
case static::TYPE_AAL2:
370+
case static::TYPE_AAL3:
371+
case static::TYPE_AAL4:
372+
case static::TYPE_AAL5:
373+
if ($adminLevels->has($adminLevelMap[$type])) {
374+
return true;
375+
}
376+
377+
break;
378+
case static::TYPE_LOC:
379+
if ($address->getLocality() !== null) {
380+
return true;
381+
}
382+
383+
break;
384+
case static::TYPE_SUBLOC:
385+
if ($address->getSubLocality() !== null) {
386+
return true;
387+
}
388+
389+
break;
390+
case static::TYPE_POSTAL:
391+
if ($address->getPostalCode() !== null) {
392+
return true;
393+
}
394+
395+
break;
396+
case static::TYPE_ADDRESS:
397+
if ($address->getStreetName() !== null) {
398+
return true;
399+
}
400+
401+
break;
402+
case static::TYPE_NUMBER:
403+
if ($address->getStreetNumber() !== null) {
404+
return true;
405+
}
406+
407+
break;
408+
}
409+
}
344410

345-
//TODO
346-
return true;
411+
return false;
347412
}
348413

349414
/**

‎src/Model/Behavior/GeocoderBehavior.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,11 +475,29 @@ public function distanceConditions($distance = null, $fieldName = null, $fieldLa
475475
if ($tableName === null) {
476476
$tableName = $this->_table->getAlias();
477477
}
478+
479+
// Validate field and table names to prevent SQL injection
480+
if (!preg_match('/^[a-zA-Z0-9_]+$/', $tableName)) {
481+
throw new \InvalidArgumentException('Invalid table name');
482+
}
483+
if (!preg_match('/^[a-zA-Z0-9_]+$/', $fieldLat)) {
484+
throw new \InvalidArgumentException('Invalid field name');
485+
}
486+
if (!preg_match('/^[a-zA-Z0-9_]+$/', $fieldLng)) {
487+
throw new \InvalidArgumentException('Invalid field name');
488+
}
489+
478490
$conditions = [
479491
$tableName . '.' . $fieldLat . ' <> 0',
480492
$tableName . '.' . $fieldLng . ' <> 0',
481493
];
482494
$fieldName = !empty($fieldName) ? $fieldName : 'distance';
495+
496+
// Validate distance field name
497+
if (!preg_match('/^[a-zA-Z0-9_]+$/', $fieldName)) {
498+
throw new \InvalidArgumentException('Invalid field name');
499+
}
500+
483501
if ($distance !== null) {
484502
$conditions[] = '1=1 HAVING ' . $tableName . '.' . $fieldName . ' < ' . (int)$distance;
485503
}

‎src/View/Helper/GoogleMapHelper.php‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ public function initialize(array $config): void {
354354
* @param array<string, mixed> $query
355355
* @return string Full URL
356356
*/
357-
public function apiUrl(array $query = []) {
357+
public function apiUrl(array $query = []): string {
358358
$url = $this->_protocol() . static::API;
359359

360360
if ($this->_runtimeConfig['map']['api']) {
@@ -381,7 +381,7 @@ public function apiUrl(array $query = []) {
381381
* @deprecated Not in use.
382382
* @return string
383383
*/
384-
public function gearsUrl() {
384+
public function gearsUrl(): string {
385385
$this->_gearsIncluded = true;
386386
$url = $this->_protocol() . 'code.google.com/apis/gears/gears_init.js';
387387

@@ -391,14 +391,14 @@ public function gearsUrl() {
391391
/**
392392
* @return string currentMapObject
393393
*/
394-
public function name() {
394+
public function name(): string {
395395
return 'map' . static::$mapCount;
396396
}
397397

398398
/**
399399
* @return string currentContainerId
400400
*/
401-
public function id() {
401+
public function id(): string {
402402
return $this->_runtimeConfig['div']['id'];
403403
}
404404

@@ -409,7 +409,7 @@ public function id() {
409409
* @param bool $full true=optionsAsWell
410410
* @return void
411411
*/
412-
public function reset($full = true) {
412+
public function reset(bool $full = true): void {
413413
static::$markerCount = static::$infoWindowCount = 0;
414414
$this->markers = $this->infoWindows = [];
415415
if ($full) {
@@ -430,7 +430,7 @@ public function reset($full = true) {
430430
* @param array<string, mixed> $options
431431
* @return void
432432
*/
433-
public function setControls(array $options = []) {
433+
public function setControls(array $options = []): void {
434434
if (isset($options['streetView'])) {
435435
$this->_runtimeConfig['map']['streetViewControl'] = $options['streetView'];
436436
}
@@ -455,7 +455,7 @@ public function setControls(array $options = []) {
455455
* @param array<string, mixed> $options associative array of settings are passed
456456
* @return string divContainer
457457
*/
458-
public function map(array $options = []) {
458+
public function map(array $options = []): string {
459459
$this->reset();
460460
$this->_runtimeConfig = Hash::merge($this->_runtimeConfig, $options);
461461
$this->_runtimeConfig['map'] = $options + $this->_runtimeConfig['map'];
@@ -560,7 +560,7 @@ protected function _initialLocation() {
560560
* @throws \Cake\Core\Exception\CakeException
561561
* @return mixed Integer marker count or boolean false on failure
562562
*/
563-
public function addMarker($options) {
563+
public function addMarker(array $options): mixed {
564564
$defaults = $this->_runtimeConfig['marker'];
565565
if (isset($options['icon']) && is_array($options['icon'])) {
566566
$defaults = $options['icon'] + $defaults;
@@ -732,7 +732,7 @@ protected function _directions($directions, array $markerOptions = []) {
732732
* @param string $content
733733
* @return int Current marker counter
734734
*/
735-
public function addInfoContent($content) {
735+
public function addInfoContent(string $content): int {
736736
$this->infoContents[static::$markerCount] = $this->escapeString($content);
737737
$event = "
738738
gWindowContents" . static::$mapCount . '.push(' . $this->escapeString($content) . ");
@@ -762,7 +762,7 @@ public function addInfoContent($content) {
762762
* NOTE: for special ones only first parameter counts!
763763
* @return array Array(icon, shadow, shape, ...)
764764
*/
765-
public function iconSet($color, $char = null, $size = 'm') {
765+
public function iconSet(string $color, ?string $char = null, string $size = 'm'): array {
766766
$colors = ['red', 'green', 'yellow', 'blue', 'purple', 'white', 'black'];
767767
if (!in_array($color, $colors)) {
768768
$color = 'red';
@@ -833,7 +833,7 @@ public function iconSet($color, $char = null, $size = 'm') {
833833
* @param array<string, mixed> $shadowOptions Shadow image options
834834
* @return array Resulting array
835835
*/
836-
public function addIcon($image, $shadow = null, array $imageOptions = [], array $shadowOptions = []) {
836+
public function addIcon(string $image, ?string $shadow = null, array $imageOptions = [], array $shadowOptions = []): array {
837837
$res = ['url' => $image];
838838
$res['icon'] = $this->icon($image, $imageOptions);
839839
if ($shadow) {
@@ -864,14 +864,18 @@ public function addIcon($image, $shadow = null, array $imageOptions = [], array
864864
* - anchor: array(width=>x, height=>y)
865865
* @return int Icon count
866866
*/
867-
public function icon(string $url, array $options = []) {
867+
public function icon(string $url, array $options = []): int {
868868
// The shadow image is larger in the horizontal dimension
869869
// while the position and offset are the same as for the main image.
870870
if (empty($options['size'])) {
871871
// We will deprecate this in the future maybe as this is super slow!
872872
//trigger_error('Please specify size manually [width => ..., height => ...] for performance reasons.', E_USER_DEPRECATED);
873873
if (!empty($options['imagePath'])) {
874874
$path = realpath($options['imagePath']);
875+
$allowedDir = realpath(WWW_ROOT . 'img' . DS);
876+
if (!$path || !$allowedDir || strpos($path, $allowedDir) !== 0) {
877+
throw new CakeException('Invalid image path');
878+
}
875879
} else {
876880
$path = $url;
877881
if (!preg_match('#^((https?://)|//)#i', $path)) {
@@ -919,7 +923,7 @@ public function icon(string $url, array $options = []) {
919923
* - lat, lng, content, maxWidth, pixelOffset, zIndex
920924
* @return int windowCount
921925
*/
922-
public function addInfoWindow(array $options = []) {
926+
public function addInfoWindow(array $options = []): int {
923927
$defaults = $this->_runtimeConfig['infoWindow'];
924928
$options += $defaults;
925929

@@ -951,7 +955,7 @@ public function addInfoWindow(array $options = []) {
951955
* @param bool $open Also open it right away.
952956
* @return void
953957
*/
954-
public function addEvent($marker, $infoWindow, $open = false) {
958+
public function addEvent(int $marker, int $infoWindow, bool $open = false): void {
955959
$this->map .= "
956960
google.maps.event.addListener(gMarkers" . static::$mapCount . "[{$marker}], 'click', function() {
957961
gInfoWindows" . static::$mapCount . "[$infoWindow].open(" . $this->name() . ", this);
@@ -971,7 +975,7 @@ public function addEvent($marker, $infoWindow, $open = false) {
971975
* @param string $event (js)
972976
* @return void
973977
*/
974-
public function addCustomEvent($marker, $event) {
978+
public function addCustomEvent(int $marker, string $event): void {
975979
$this->map .= "
976980
google.maps.event.addListener(gMarkers" . static::$mapCount . "[{$marker}], 'click', function() {
977981
$event
@@ -985,7 +989,7 @@ public function addCustomEvent($marker, $event) {
985989
* @param string $js Custom JS
986990
* @return void
987991
*/
988-
public function addCustom($js) {
992+
public function addCustom(string $js): void {
989993
$this->map .= $js;
990994
}
991995

@@ -1008,7 +1012,7 @@ public function addCustom($js) {
10081012
* - region: String
10091013
* @return void
10101014
*/
1011-
public function addDirections($from, $to, array $options = []) {
1015+
public function addDirections(array|string $from, array|string $to, array $options = []): void {
10121016
$id = 'd' . static::$markerCount++;
10131017
$defaults = $this->_runtimeConfig['directions'];
10141018
$options += $defaults;
@@ -1066,7 +1070,7 @@ public function addDirections($from, $to, array $options = []) {
10661070
* - weight in pixels (defaults to 2)
10671071
* @return void
10681072
*/
1069-
public function addPolyline($from, $to, array $options = []) {
1073+
public function addPolyline(array|string $from, array|string $to, array $options = []): void {
10701074
if (is_array($from)) {
10711075
$from = 'new google.maps.LatLng(' . (float)$from['lat'] . ', ' . (float)$from['lng'] . ')';
10721076
} else {
@@ -1110,7 +1114,7 @@ public function addPolyline($from, $to, array $options = []) {
11101114
* @param int $index infoWindowCount
11111115
* @return void
11121116
*/
1113-
public function setContentInfoWindow($content, $index) {
1117+
public function setContentInfoWindow(string $content, int $index): void {
11141118
$this->map .= "
11151119
gInfoWindows" . static::$mapCount . "[$index]. setContent(" . $this->escapeString($content) . ');';
11161120
}
@@ -1121,7 +1125,7 @@ public function setContentInfoWindow($content, $index) {
11211125
* @param mixed $content
11221126
* @return string JSON
11231127
*/
1124-
public function escapeString($content) {
1128+
public function escapeString(mixed $content): string {
11251129
$result = json_encode($content);
11261130
if ($result === false) {
11271131
return '';

0 commit comments

Comments
 (0)