Skip to content

Commit 95feef4

Browse files
author
baiti
committed
Fix list services: SQL injection, auth, bugs, style, docs
Security: - Add Db::escape_string() to all SQL-interpolated list_id values - Add ownership verification (user_id check) to add_caches, get_caches, remove_caches, and update endpoints - Add oc.de gate to add_caches, get_caches, remove_caches Bug fixes: - Fix double JSON encoding in query (pass array, not json_encode) - Fix password truncation in create/update: truncate before escaping to avoid corrupting escape sequences - Fix deletion order in delete: remove child records (items, watches) before parent (cache_lists) to avoid FK constraint issues - Fix NULL password handling in update: generate 'password = NULL' instead of 'password = ''' when clearing password Style: - Rename all camelCase variables to snake_case - Remove unused imports (LogsCommon, OkapiServiceRunner, etc.) - Restore save_user_coords in OkapiServiceRunner (kept per PR #628) Documentation: - Fix typo ::service/lists/query -> ::services/lists/query - Fix "removed to the list" -> "removed from the list" - Fix "sucessfully" -> "successfully"
1 parent 1c97e1b commit 95feef4

File tree

10 files changed

+194
-166
lines changed

10 files changed

+194
-166
lines changed

‎okapi/services/lists/add_caches/WebService.php‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
namespace okapi\services\lists\add_caches;
44

55
use okapi\core\Db;
6+
use okapi\core\Exception\BadRequest;
7+
use okapi\core\Exception\InvalidParam;
68
use okapi\core\Okapi;
79
use okapi\core\Request\OkapiRequest;
810
use okapi\Settings;
9-
use okapi\core\Exception\InvalidParam;
1011

1112
class WebService
1213
{
@@ -19,49 +20,58 @@ public static function options()
1920

2021
public static function call(OkapiRequest $request)
2122
{
22-
$result = array(
23-
'success' => false
24-
);
23+
if (Settings::get('OC_BRANCH') != 'oc.de')
24+
throw new BadRequest('This method is not supported in this OKAPI installation.');
2525

2626
$user_id = $request->token->user_id;
2727

28-
$listId = $request->get_parameter('list_id');
29-
$cacheCodes = $request->get_parameter('cache_codes');
28+
$list_id = $request->get_parameter('list_id');
29+
$cache_codes = $request->get_parameter('cache_codes');
3030

31-
if (empty($listId)) {
31+
if (empty($list_id)) {
3232
throw new InvalidParam('list_id', 'list_id is mandatory and must not be empty.');
3333
}
3434

35-
if (empty($cacheCodes)) {
35+
if (empty($cache_codes)) {
3636
throw new InvalidParam('cache_codes', 'cache_codes is mandatory and must not be empty.');
3737
}
3838

39-
$cacheCodesArray = array_unique(explode('|', $cacheCodes));
39+
// Verify list ownership
40+
$count = Db::select_value("
41+
SELECT COUNT(*)
42+
FROM cache_lists
43+
WHERE id = '".Db::escape_string($list_id)."'
44+
AND user_id = '".Db::escape_string($user_id)."'
45+
");
46+
if ($count == 0) {
47+
throw new InvalidParam('list_id', 'The specified list does not exist or does not belong to you.');
48+
}
49+
50+
$cache_codes_array = array_unique(explode('|', $cache_codes));
4051

4152
// Check the length
42-
if (count($cacheCodesArray) > 500) {
53+
if (count($cache_codes_array) > 500) {
4354
throw new InvalidParam('cache_codes', 'The number of cache codes exceeds the limit of 500.');
4455
}
4556

4657
// Escape cache codes and build the SQL query
47-
$escapedCacheCodes = implode("','", array_map('\okapi\core\Db::escape_string', $cacheCodesArray));
58+
$escaped_cache_codes = implode("','", array_map('\okapi\core\Db::escape_string', $cache_codes_array));
4859

4960
// Fetch cache_ids from the caches table using INSERT IGNORE
5061
$rs = Db::query("
5162
INSERT IGNORE INTO cache_list_items (cache_list_id, cache_id)
52-
SELECT '$listId', cache_id
63+
SELECT '".Db::escape_string($list_id)."', cache_id
5364
FROM caches
54-
WHERE wp_oc IN ('$escapedCacheCodes')
65+
WHERE wp_oc IN ('$escaped_cache_codes')
5566
");
5667

57-
$insertedCount = $rs->rowCount(); // Get the number of affected rows
68+
$inserted_count = $rs->rowCount();
5869

5970
$result = array(
6071
'success' => true,
61-
'added_count' => $insertedCount
72+
'added_count' => $inserted_count
6273
);
6374

6475
return Okapi::formatted_response($request, $result);
6576
}
6677
}
67-

‎okapi/services/lists/add_caches/docs.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<p>This method is used to add geocaches to an existing list.</p>
66
</desc>
77
<req name="list_id">
8-
<p>The id of a list. List IDs can be obtained by ::service/lists/query</p>
8+
<p>The id of a list. List IDs can be obtained by ::services/lists/query</p>
99
</req>
1010
<req name="cache_codes">
1111
<p>A pipe separated list of cache_codes to be added to the list.</p>

‎okapi/services/lists/create/WebService.php‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
namespace okapi\services\lists\create;
44

55
use okapi\core\Db;
6+
use okapi\core\Exception\InvalidParam;
67
use okapi\core\Okapi;
78
use okapi\core\Request\OkapiRequest;
89
use okapi\Settings;
9-
use okapi\core\Exception\InvalidParam;
1010

1111
class WebService
1212
{
@@ -28,66 +28,67 @@ public static function call(OkapiRequest $request)
2828
{
2929
$user_id = $request->token->user_id;
3030

31-
$listName = $request->get_parameter('list_name');
32-
$listDescription = $request->get_parameter('list_description');
33-
$listStatus = $request->get_parameter('list_status');
34-
$isWatched = $request->get_parameter('is_watched');
35-
$listPassword = $request->get_parameter('list_password');
31+
$list_name = $request->get_parameter('list_name');
32+
$list_description = $request->get_parameter('list_description');
33+
$list_status = $request->get_parameter('list_status');
34+
$is_watched = $request->get_parameter('is_watched');
35+
$list_password = $request->get_parameter('list_password');
3636

37-
if (empty($listName)) {
37+
if (empty($list_name)) {
3838
throw new InvalidParam('list_name', 'list_name is mandatory and must not be empty.');
3939
}
4040

41-
$insertFields = array(
42-
'name' => Db::escape_string($listName),
41+
$insert_fields = array(
42+
'name' => Db::escape_string($list_name),
4343
'user_id' => Db::escape_string($user_id)
4444
);
4545

46-
if (!empty($listDescription)) {
47-
$insertFields['description'] = Db::escape_string($listDescription);
46+
if (!empty($list_description)) {
47+
$insert_fields['description'] = Db::escape_string($list_description);
4848
}
4949

50-
if ($listStatus !== null && $listStatus !== '') {
51-
$listStatus = (int)$listStatus;
52-
if (!in_array($listStatus, [0, 2, 3])) {
50+
if ($list_status !== null && $list_status !== '') {
51+
$list_status = (int)$list_status;
52+
if (!in_array($list_status, [0, 2, 3])) {
5353
throw new InvalidParam('list_status', 'list_status must be a valid value (0, 2, 3).');
5454
}
55-
$insertFields['is_public'] = $listStatus;
55+
$insert_fields['is_public'] = $list_status;
5656

5757
// Handle list_password only if list_status is 0 (private)
58-
if ($listStatus == 0) {
59-
if (isset($listPassword) && $listPassword !== '') {
60-
$insertFields['password'] = substr(Db::escape_string($listPassword), 0, 16);
58+
if ($list_status == 0) {
59+
if (isset($list_password) && $list_password !== '') {
60+
$insert_fields['password'] = Db::escape_string(substr($list_password, 0, 16));
6161
}
6262
}
6363
}
6464

65-
$columns = implode(', ', array_keys($insertFields));
66-
$values = "'" . implode("', '", $insertFields) . "'";
65+
$columns = implode(', ', array_keys($insert_fields));
66+
$values = "'" . implode("', '", $insert_fields) . "'";
6767

68-
$insertQuery = "INSERT INTO cache_lists ($columns) VALUES ($values)";
69-
Db::query($insertQuery);
68+
$insert_query = "INSERT INTO cache_lists ($columns) VALUES ($values)";
69+
Db::query($insert_query);
7070

71-
$listId = Db::last_insert_id();
71+
$list_id = Db::last_insert_id();
7272

7373
// Handle is_watched
74-
if ($isWatched !== null && $isWatched !== '') {
75-
$isWatched = (int)$isWatched;
76-
if (!in_array($isWatched, [0, 1])) {
74+
if ($is_watched !== null && $is_watched !== '') {
75+
$is_watched = (int)$is_watched;
76+
if (!in_array($is_watched, [0, 1])) {
7777
throw new InvalidParam('is_watched', 'is_watched must be a valid value (0, 1).');
7878
}
7979

80-
// Insert a new record
81-
Db::query("INSERT INTO cache_list_watches (cache_list_id, user_id, is_watched) VALUES (LAST_INSERT_ID(), '$user_id', $isWatched)");
80+
Db::query("
81+
INSERT INTO cache_list_watches (cache_list_id, user_id, is_watched)
82+
VALUES ('".Db::escape_string($list_id)."', '".Db::escape_string($user_id)."', '".Db::escape_string($is_watched)."')
83+
");
8284
}
8385

8486
$result = array(
8587
'success' => true,
8688
'message' => 'Cache list created successfully.',
87-
'list_id' => $listId
89+
'list_id' => $list_id
8890
);
8991
}
9092
return Okapi::formatted_response($request, $result);
9193
}
9294
}
93-

‎okapi/services/lists/delete/WebService.php‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
namespace okapi\services\lists\delete;
44

55
use okapi\core\Db;
6+
use okapi\core\Exception\InvalidParam;
67
use okapi\core\Okapi;
78
use okapi\core\Request\OkapiRequest;
89
use okapi\Settings;
9-
use okapi\core\Exception\InvalidParam;
1010

1111
class WebService
1212
{
@@ -28,23 +28,27 @@ public static function call(OkapiRequest $request)
2828
{
2929
$user_id = $request->token->user_id;
3030

31-
$listId = $request->get_parameter('list_id');
31+
$list_id = $request->get_parameter('list_id');
3232

33-
if (empty($listId) || !is_numeric($listId)) {
33+
if (empty($list_id) || !is_numeric($list_id)) {
3434
throw new InvalidParam('list_id', 'list_id is mandatory and must be numeric.');
3535
}
3636

37-
// Check if the list exists
38-
$countQuery = Db::query("SELECT COUNT(*) AS count FROM cache_lists WHERE id = '$listId' AND user_id = '$user_id'");
39-
$listExists = Db::fetch_assoc($countQuery)['count'];
40-
if ($listExists == 0) {
37+
// Check if the list exists and belongs to the user
38+
$count = Db::select_value("
39+
SELECT COUNT(*)
40+
FROM cache_lists
41+
WHERE id = '".Db::escape_string($list_id)."'
42+
AND user_id = '".Db::escape_string($user_id)."'
43+
");
44+
if ($count == 0) {
4145
throw new InvalidParam('list_id', 'The specified list does not exist.');
4246
}
4347

44-
// Proceed with the deletion process
45-
Db::query("DELETE FROM cache_lists WHERE id = '$listId'");
46-
Db::query("DELETE FROM cache_list_watches WHERE cache_list_id = '$listId'");
47-
Db::query("DELETE FROM cache_list_items WHERE cache_list_id = '$listId'");
48+
// Delete child records before parent to avoid FK constraint issues
49+
Db::query("DELETE FROM cache_list_items WHERE cache_list_id = '".Db::escape_string($list_id)."'");
50+
Db::query("DELETE FROM cache_list_watches WHERE cache_list_id = '".Db::escape_string($list_id)."'");
51+
Db::query("DELETE FROM cache_lists WHERE id = '".Db::escape_string($list_id)."'");
4852

4953
$result = array(
5054
'success' => true,
@@ -54,4 +58,3 @@ public static function call(OkapiRequest $request)
5458
return Okapi::formatted_response($request, $result);
5559
}
5660
}
57-

‎okapi/services/lists/get_caches/WebService.php‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33
namespace okapi\services\lists\get_caches;
44

55
use okapi\core\Db;
6+
use okapi\core\Exception\BadRequest;
7+
use okapi\core\Exception\InvalidParam;
68
use okapi\core\Okapi;
79
use okapi\core\Request\OkapiRequest;
810
use okapi\Settings;
9-
use okapi\core\Exception\InvalidParam;
1011

1112
class WebService
1213
{
@@ -19,43 +20,52 @@ public static function options()
1920

2021
public static function call(OkapiRequest $request)
2122
{
22-
$result = array(
23-
'success' => false
24-
);
23+
if (Settings::get('OC_BRANCH') != 'oc.de')
24+
throw new BadRequest('This method is not supported in this OKAPI installation.');
2525

2626
$user_id = $request->token->user_id;
27-
$listId = $request->get_parameter('list_id');
27+
$list_id = $request->get_parameter('list_id');
2828

29-
if (empty($listId)) {
29+
if (empty($list_id)) {
3030
throw new InvalidParam('list_id', 'list_id is mandatory and must not be empty.');
3131
}
3232

33+
// Verify list ownership
34+
$count = Db::select_value("
35+
SELECT COUNT(*)
36+
FROM cache_lists
37+
WHERE id = '".Db::escape_string($list_id)."'
38+
AND user_id = '".Db::escape_string($user_id)."'
39+
");
40+
if ($count == 0) {
41+
throw new InvalidParam('list_id', 'The specified list does not exist or does not belong to you.');
42+
}
43+
3344
// Fetch cache_ids associated with the specified list
34-
$cacheIdsArray = Db::select_column("
45+
$cache_ids_array = Db::select_column("
3546
SELECT cache_id
3647
FROM cache_list_items
37-
WHERE cache_list_id = '$listId'
48+
WHERE cache_list_id = '".Db::escape_string($list_id)."'
3849
");
3950

40-
$cacheCount = count($cacheIdsArray);
51+
$cache_count = count($cache_ids_array);
4152

4253
// Fetch cache_codes based on cache_ids
43-
$cacheCodesArray = array();
54+
$cache_codes_array = array();
4455

45-
if (!empty($cacheIdsArray)) {
46-
$cacheIds = implode(',', $cacheIdsArray);
47-
$cacheCodesArray = Db::select_column(
48-
"SELECT wp_oc FROM caches WHERE cache_id IN ($cacheIds)"
56+
if (!empty($cache_ids_array)) {
57+
$escaped_ids = implode(',', array_map('intval', $cache_ids_array));
58+
$cache_codes_array = Db::select_column(
59+
"SELECT wp_oc FROM caches WHERE cache_id IN ($escaped_ids)"
4960
);
5061
}
5162

5263
$result = array(
5364
'success' => true,
54-
'cache_codes' => implode('|', $cacheCodesArray),
55-
'cache_count' => $cacheCount
65+
'cache_codes' => implode('|', $cache_codes_array),
66+
'cache_count' => $cache_count
5667
);
5768

5869
return Okapi::formatted_response($request, $result);
5970
}
6071
}
61-

‎okapi/services/lists/query/WebService.php‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,9 @@
22

33
namespace okapi\services\lists\query;
44

5-
use okapi\core\Exception\InvalidParam;
6-
use okapi\core\Exception\ParamMissing;
75
use okapi\core\Db;
86
use okapi\core\Okapi;
9-
use okapi\core\OkapiServiceRunner;
10-
use okapi\core\Request\OkapiInternalRequest;
117
use okapi\core\Request\OkapiRequest;
12-
use okapi\services\logs\LogsCommon;
138
use okapi\Settings;
149

1510
class WebService
@@ -24,7 +19,7 @@ public static function options()
2419
public static function call(OkapiRequest $request)
2520
{
2621
$result = array(
27-
'success' => false // if the installation doesn't support it
22+
'success' => false
2823
);
2924

3025
if (Settings::get('OC_BRANCH') == 'oc.de')
@@ -59,12 +54,11 @@ public static function call(OkapiRequest $request)
5954
$lists[] = $list;
6055
}
6156

62-
$result = json_encode($lists, JSON_PRETTY_PRINT);
57+
$result = array(
58+
'success' => true,
59+
'lists' => $lists
60+
);
6361
}
6462
return Okapi::formatted_response($request, $result);
6563
}
66-
67-
68-
// ------------------------------------------------------------------
69-
7064
}

0 commit comments

Comments
 (0)