From c8a3c58495e06f16745154508db4619b642c5d48 Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Tue, 23 Jun 2026 12:07:03 -0500 Subject: [PATCH] =?UTF-8?q?fix:=20code=20review=20fixes=20=E2=80=94=20secu?= =?UTF-8?q?rity,=200-coord=20bug,=20BOM,=20ACL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - ACL check (core.create) in ImportController before processing - File extension validation (.csv/.txt only) on upload - Website href restricted to http/https scheme (prevents javascript: XSS) Bug fixes: - Fix 0.0 coordinate rejection: use null checks instead of != 0.0 (coordinates at equator/prime meridian are valid locations) - Fix Haversine guard using !== null instead of PHP truthiness - Fix geocoding result check: isset+is_numeric instead of !empty - Strip UTF-8 BOM from first CSV header (fixes Excel-generated imports) - Cap radius at 25000 to prevent unreasonable distance queries Authored-by: Moko Consulting --- .../admin/src/Controller/ImportController.php | 20 ++++++++++++++ .../admin/src/Model/ImportModel.php | 7 ++++- .../admin/src/Model/LocationModel.php | 3 ++- .../site/src/Model/LocationsModel.php | 26 +++++++++++++------ .../site/tmpl/location/default.php | 2 +- 5 files changed, 47 insertions(+), 11 deletions(-) diff --git a/source/packages/com_mokosuitestorelocator/admin/src/Controller/ImportController.php b/source/packages/com_mokosuitestorelocator/admin/src/Controller/ImportController.php index 79519d8..9f309e5 100644 --- a/source/packages/com_mokosuitestorelocator/admin/src/Controller/ImportController.php +++ b/source/packages/com_mokosuitestorelocator/admin/src/Controller/ImportController.php @@ -33,6 +33,15 @@ class ImportController extends BaseController { Session::checkToken() or jexit(Text::_('JINVALID_TOKEN')); + // ACL check — user must have create permission + if (!$this->app->getIdentity()->authorise('core.create', 'com_mokosuitestorelocator')) + { + $this->setMessage(Text::_('JLIB_APPLICATION_ERROR_CREATE_RECORD_NOT_PERMITTED'), 'error'); + $this->setRedirect(Route::_('index.php?option=com_mokosuitestorelocator&view=locations', false)); + + return; + } + /** @var \Moko\Component\MokoSuiteStoreLocator\Administrator\Model\ImportModel $model */ $model = $this->getModel('Import', 'Administrator'); @@ -49,6 +58,17 @@ class ImportController extends BaseController return; } + // Validate file extension + $ext = strtolower(pathinfo($csvFile['name'], PATHINFO_EXTENSION)); + + if ($ext !== 'csv' && $ext !== 'txt') + { + $this->setMessage(Text::_('COM_MOKOJOOMSTORELOCATOR_IMPORT_INVALID_FILE'), 'error'); + $this->setRedirect(Route::_('index.php?option=com_mokosuitestorelocator&view=import', false)); + + return; + } + $result = $model->processImport($csvFile['tmp_name'], $delimiter); if ($result['imported'] > 0) diff --git a/source/packages/com_mokosuitestorelocator/admin/src/Model/ImportModel.php b/source/packages/com_mokosuitestorelocator/admin/src/Model/ImportModel.php index 4fcdc63..e859cb1 100644 --- a/source/packages/com_mokosuitestorelocator/admin/src/Model/ImportModel.php +++ b/source/packages/com_mokosuitestorelocator/admin/src/Model/ImportModel.php @@ -77,9 +77,14 @@ class ImportModel extends BaseDatabaseModel $file->setFlags(SplFileObject::READ_CSV | SplFileObject::SKIP_EMPTY | SplFileObject::DROP_NEW_LINE); $file->setCsvControl($delimiter); - // Read and map headers + // Read and map headers (strip UTF-8 BOM from Excel exports) $headers = $file->fgetcsv(); + if (!empty($headers[0])) + { + $headers[0] = ltrim($headers[0], "\xEF\xBB\xBF"); + } + if (!$headers || \count($headers) < 2) { $result['errors'][] = 'Invalid CSV headers.'; diff --git a/source/packages/com_mokosuitestorelocator/admin/src/Model/LocationModel.php b/source/packages/com_mokosuitestorelocator/admin/src/Model/LocationModel.php index 3b75653..bbbc150 100644 --- a/source/packages/com_mokosuitestorelocator/admin/src/Model/LocationModel.php +++ b/source/packages/com_mokosuitestorelocator/admin/src/Model/LocationModel.php @@ -160,7 +160,8 @@ class LocationModel extends AdminModel $results = json_decode($response->body, true); - if (!empty($results[0]['lat']) && !empty($results[0]['lon'])) + if (isset($results[0]['lat']) && is_numeric($results[0]['lat']) + && isset($results[0]['lon']) && is_numeric($results[0]['lon'])) { return [ 'lat' => round((float) $results[0]['lat'], 8), diff --git a/source/packages/com_mokosuitestorelocator/site/src/Model/LocationsModel.php b/source/packages/com_mokosuitestorelocator/site/src/Model/LocationsModel.php index 9e395ba..6d44b90 100644 --- a/source/packages/com_mokosuitestorelocator/site/src/Model/LocationsModel.php +++ b/source/packages/com_mokosuitestorelocator/site/src/Model/LocationsModel.php @@ -67,22 +67,32 @@ class LocationsModel extends ListModel $state = $app->input->getString('state', ''); $this->setState('filter.state', $state); - $lat = $app->input->getFloat('lat', 0.0); - $lng = $app->input->getFloat('lng', 0.0); + $latRaw = $app->input->getString('lat', ''); + $lngRaw = $app->input->getString('lng', ''); $radius = $app->input->getInt('radius', 0); $radiusUnit = $app->input->getString('radius_unit', 'miles'); - if ($lat >= -90 && $lat <= 90 && $lat != 0.0) + if ($latRaw !== '' && is_numeric($latRaw)) { - $this->setState('filter.lat', $lat); + $lat = (float) $latRaw; + + if ($lat >= -90 && $lat <= 90) + { + $this->setState('filter.lat', $lat); + } } - if ($lng >= -180 && $lng <= 180 && $lng != 0.0) + if ($lngRaw !== '' && is_numeric($lngRaw)) { - $this->setState('filter.lng', $lng); + $lng = (float) $lngRaw; + + if ($lng >= -180 && $lng <= 180) + { + $this->setState('filter.lng', $lng); + } } - if ($radius > 0) + if ($radius > 0 && $radius <= 25000) { $this->setState('filter.radius', $radius); } @@ -149,7 +159,7 @@ class LocationsModel extends ListModel $lng = $this->getState('filter.lng'); $radius = $this->getState('filter.radius'); - if ($lat && $lng && $radius) + if ($lat !== null && $lng !== null && $radius) { $unit = $this->getState('filter.radius_unit', 'miles'); $earthRadius = ($unit === 'km') ? 6371 : 3959; diff --git a/source/packages/com_mokosuitestorelocator/site/tmpl/location/default.php b/source/packages/com_mokosuitestorelocator/site/tmpl/location/default.php index bd41e58..f6463e5 100644 --- a/source/packages/com_mokosuitestorelocator/site/tmpl/location/default.php +++ b/source/packages/com_mokosuitestorelocator/site/tmpl/location/default.php @@ -86,7 +86,7 @@ $item = $this->item; - website) : ?> + website && preg_match('#^https?://#i', $item->website)) : ?>
: