Deep scan: duplicate curl, type bugs, and security hardening across service plugins #146

New Issue

2026-06-21T22:55:19Z

jmiller commented

2026-06-21 22:55:19 +00:00

Summary

Deep code scan found additional bugs and security improvements needed. These are the same class of issues as #139 (which fixed SendGrid, Reddit, TikTok, Pinterest) but in 3 more plugins, plus new findings.

Bugs (CRITICAL)

1. Duplicate curl_setopt_array + undefined `$token` — 3 more plugins

Same pattern as #139 but in plugins that were missed:

Plugin	File	Lines	Variable Bug
ConvertKit	`plg_mokosuitecross_convertkit/src/Extension/ConvertkitService.php`	60+69	`$token` undefined (should be `$apiSecret`)
Brevo	`plg_mokosuitecross_brevo/src/Extension/BrevoService.php`	64+73	`$token` undefined + wrong auth header (should be `api-key:` not `Bearer`)
Constant Contact	`plg_mokosuitecross_constantcontact/src/Extension/ConstantcontactService.php`	67+76	Identical duplicate (no variable bug)

Fix: Remove the second curl_setopt_array() call in each plugin's publish() method.

2. Medium `getUserId()` returns array instead of string

File: plg_mokosuitecross_medium/src/Extension/MediumService.php:166

Method declares return string but the error path returns ['valid' => false, ...] — an array copy-pasted from validateCredentials(). Will cause PHP type error.

Fix: Change line 166 to return '';

3. Mailchimp incorrect HTTP status check

File: plg_mokosuitecross_mailchimp/src/Extension/MailchimpService.php:98

if ($httpCode !== 200 || empty($data['id'])) {

Mailchimp campaign creation returns 201 Created, not 200. This means campaign creation always fails even with valid credentials.

Fix: Change to if ($httpCode < 200 || $httpCode >= 300 || empty($data['id']))

Security Hardening (MEDIUM)

4. Exception details exposed in ServiceController

File: com_mokosuitecross/src/Controller/ServiceController.php:95-100

echo new JsonResponse($e);

Full exception with stack trace exposed to client. Should catch and return generic error message.

5. DispatchController missing CSRF check

File: com_mokosuitecross/src/Controller/DispatchController.php:47

POST endpoint modifies database state without $this->checkToken(). Note: This is a REST API endpoint — may intentionally skip CSRF for API consumers, but should be documented or use API token auth instead.

6. SQL WHERE IN uses implode instead of whereIn()

Files: DispatchController.php:112, CrossPostDispatcher.php:128,146, QueueProcessor.php:351-375

All use implode(',', $ids) in SQL WHERE IN clauses. Values are cast to int so not exploitable, but should use Joomla's $query->whereIn() for consistency.

7. Bluesky MD5 cache key

File: plg_mokosuitecross_bluesky/src/Extension/BlueskyService.php:130

Uses md5() for cache key generation. Not a security risk (just cache indexing) but should use hash('sha256', ...).

Acceptance Criteria

Duplicate curl_setopt_array removed from ConvertKit, Brevo, Constant Contact
Medium getUserId() returns '' on error instead of array
Mailchimp checks $httpCode >= 200 && $httpCode < 300
ServiceController catches exceptions and returns generic error
SQL uses whereIn() instead of implode
Bluesky uses hash('sha256', ...) instead of md5()

## Summary Deep code scan found additional bugs and security improvements needed. These are the same class of issues as #139 (which fixed SendGrid, Reddit, TikTok, Pinterest) but in 3 more plugins, plus new findings. ## Bugs (CRITICAL) ### 1. Duplicate curl_setopt_array + undefined `$token` — 3 more plugins Same pattern as #139 but in plugins that were missed: | Plugin | File | Lines | Variable Bug | |--------|------|-------|-------------| | **ConvertKit** | `plg_mokosuitecross_convertkit/src/Extension/ConvertkitService.php` | 60+69 | `$token` undefined (should be `$apiSecret`) | | **Brevo** | `plg_mokosuitecross_brevo/src/Extension/BrevoService.php` | 64+73 | `$token` undefined + wrong auth header (should be `api-key:` not `Bearer`) | | **Constant Contact** | `plg_mokosuitecross_constantcontact/src/Extension/ConstantcontactService.php` | 67+76 | Identical duplicate (no variable bug) | **Fix**: Remove the second `curl_setopt_array()` call in each plugin's `publish()` method. ### 2. Medium `getUserId()` returns array instead of string **File**: `plg_mokosuitecross_medium/src/Extension/MediumService.php:166` Method declares `return string` but the error path returns `['valid' => false, ...]` — an array copy-pasted from `validateCredentials()`. Will cause PHP type error. **Fix**: Change line 166 to `return '';` ### 3. Mailchimp incorrect HTTP status check **File**: `plg_mokosuitecross_mailchimp/src/Extension/MailchimpService.php:98` ```php if ($httpCode !== 200 || empty($data['id'])) { ``` Mailchimp campaign creation returns `201 Created`, not `200`. This means campaign creation always fails even with valid credentials. **Fix**: Change to `if ($httpCode < 200 || $httpCode >= 300 || empty($data['id']))` ## Security Hardening (MEDIUM) ### 4. Exception details exposed in ServiceController **File**: `com_mokosuitecross/src/Controller/ServiceController.php:95-100` ```php echo new JsonResponse($e); ``` Full exception with stack trace exposed to client. Should catch and return generic error message. ### 5. DispatchController missing CSRF check **File**: `com_mokosuitecross/src/Controller/DispatchController.php:47` POST endpoint modifies database state without `$this->checkToken()`. Note: This is a REST API endpoint — may intentionally skip CSRF for API consumers, but should be documented or use API token auth instead. ### 6. SQL WHERE IN uses implode instead of whereIn() **Files**: `DispatchController.php:112`, `CrossPostDispatcher.php:128,146`, `QueueProcessor.php:351-375` All use `implode(',', $ids)` in SQL WHERE IN clauses. Values are cast to int so not exploitable, but should use Joomla's `$query->whereIn()` for consistency. ### 7. Bluesky MD5 cache key **File**: `plg_mokosuitecross_bluesky/src/Extension/BlueskyService.php:130` Uses `md5()` for cache key generation. Not a security risk (just cache indexing) but should use `hash('sha256', ...)`. ## Acceptance Criteria - [ ] Duplicate `curl_setopt_array` removed from ConvertKit, Brevo, Constant Contact - [ ] Medium `getUserId()` returns `''` on error instead of array - [ ] Mailchimp checks `$httpCode >= 200 && $httpCode < 300` - [ ] ServiceController catches exceptions and returns generic error - [ ] SQL uses `whereIn()` instead of `implode` - [ ] Bluesky uses `hash('sha256', ...)` instead of `md5()`

jmiller commented

2026-06-21 22:55:24 +00:00

Branch created: feature/146-deep-scan-duplicate-curl-type-bugs-and-s

git fetch origin
git checkout feature/146-deep-scan-duplicate-curl-type-bugs-and-s

Branch created: [`feature/146-deep-scan-duplicate-curl-type-bugs-and-s`](https://git.mokoconsulting.tech/MokoConsulting/MokoSuiteCross/src/branch/feature/146-deep-scan-duplicate-curl-type-bugs-and-s) ```bash git fetch origin git checkout feature/146-deep-scan-duplicate-curl-type-bugs-and-s ```

jmiller referenced this issue from a commit

2026-06-21 22:59:16 +00:00

fix: deep scan bugs — duplicate curl, type error, status check, security (#146)

jmiller closed this issue

2026-06-21 23:03:53 +00:00

jmiller referenced this issue

2026-06-21 23:04:08 +00:00

Test: deep scan fixes — curl bugs, type errors, namespace, security hardening #147

Sign in to join this conversation.