# Patch Analysis — CVE-2026-10835 (SALESmanago & Leadoo WordPress plugin)

## Scope of analysis

This document analyzes the fix shipped in plugin version **3.11.3** (the version
that remediates CVE-2026-10835) against the vulnerability reproduced in the
parent stage via the `salesmanago_export_count_contacts` AJAX action, and
evaluates whether the fix covers the materially distinct entry/data paths that
could reach the same SQL-injection sink. The comparison is based on a
line-by-line diff of the vulnerable **3.11.2** and fixed **3.11.3** source trees
(both downloaded as official WordPress.org plugin zips:
`https://downloads.wordpress.org/plugin/salesmanago.{3.11.2,3.11.3}.zip`).

## What the fix changes (files, functions, logic)

### 1. `src/Includes/SecureHelper.php` — `validate_ajax_nonce()` (authorization)
**Vulnerable (3.11.2), lines 81–104:**
```php
public static function validate_ajax_nonce($action) {
    if ( function_exists( 'current_user_can' ) && ! current_user_can( 'manage_options' ) ) {
        return false;                       // <-- early return, NO die()
    }
    $actions_requiring_nonce = [ 'salesmanago_export_count_contacts',
        'salesmanago_export_contacts', 'salesmanago_export_count_events',
        'salesmanago_export_events', 'salesmanago_export_products' ];
    if (! in_array($action, $actions_requiring_nonce, true)) {
        return false;                       // <-- return false, NO die()
    }
    if ( function_exists( 'check_ajax_referer' ) ) {
        check_ajax_referer( 'salesmanago_admin', 'sm_nonce' );   // dies on bad nonce
    }
    if ( ! current_user_can( 'manage_options' ) ) {
        wp_send_json_error( array( 'message' => 'forbidden' ), 403 );   // dies
    }
}
```
The defect: for any non-admin user the function returns `false` at the very top
**without calling `die()`/`wp_send_json_error()`**, and every caller ignores the
return value. So an authenticated Subscriber proceeds past the guard with no
nonce.

**Fixed (3.11.3), lines 88–99:**
```php
private const AJAX_ACTIONS = array(
    'salesmanago_export_count_contacts','salesmanago_export_contacts',
    'salesmanago_export_count_events','salesmanago_export_events',
    'salesmanago_export_products'
);
public static function validate_ajax_nonce($action) {
    $request_action = sanitize_text_field( $_REQUEST['action'] ?? '' );
    if (
        $request_action !== $action
        || ! in_array($request_action, self::AJAX_ACTIONS, true)
        || ! check_ajax_referer( 'salesmanago_admin', 'sm_nonce', false )
        || ! current_user_can( 'manage_options' )
    ) {
        wp_send_json_error( array( 'message' => 'forbidden' ), 403 );   // DIES
    }
}
```
The fix is a single combined predicate that **dies with HTTP 403** if any of:
the request action differs from the expected action, the action is not in the
allow-list, the nonce is invalid, **or** the user lacks `manage_options`. This
closes the early-return-without-die defect for every export AJAX action.

### 2. `src/Admin/Controller/ExportController.php` — per-handler capability gates
In 3.11.3 every export handler (`countContacts`, `exportContacts`, `countEvents`,
`exportEvents`, `exportProducts`) gains an explicit redundant gate immediately
after `validate_ajax_nonce()`:
```php
if ( ! current_user_can( 'manage_options' ) ) {
    $this->ExportModel->setMessage( 'Access denied' );
    $this->ExportModel->setStatus( self::FAILED );
    $this->ExportModel->buildResponse();   // (or buildProductExportResponse)
    return;
}
```
Additionally, `exportContacts()` had a pre-existing typo where it called
`validate_ajax_nonce( 'salesmanago_count_contacts' )` (a non-existent action
name). In 3.11.2 this was harmless to a Subscriber (early-return false either
way) but in 3.11.3 the new `$request_action !== $action` check would reject a
legitimate admin too; the fix therefore corrects the call to
`validate_ajax_nonce( 'salesmanago_export_contacts' )` (line 140).

### 3. `src/Admin/Model/ExportModel.php` — `parseArgs()` (input validation)
**Vulnerable (3.11.2), lines 138–148:**
```php
$data = json_decode( base64_decode( $_REQUEST['data'] ) );
$this->dateFrom = empty( $data->dateFrom ) ? '2000-01-01' : $data->dateFrom;   // NO sanitization
$this->dateTo   = empty( $data->dateTo )   ? date('Y-m-d', time()+86400)
                                           : date('Y-m-d', strtotime( $data->dateTo ) + 86400 );
```
**Fixed (3.11.3), lines 137–165:** adds `isset` check on `$_REQUEST['data']`,
`sanitize_text_field()` on the raw base64, strict `base64_decode(..., true)`
with false-check, `json_last_error()` validation, and routes `dateFrom`/`dateTo`
through a new `validate_date_format( sanitize_text_field( $raw ) )` helper
(defined at line 907) that enforces a strict date pattern. `identifierType`,
`statuses`, and `exportAs` are also sanitized/whitelisted.

### 4. `src/Admin/Model/ExportModel.php` — `getExportContactsQuery()` (the SQL sink)
**Vulnerable (3.11.2), lines 401–406:** direct string interpolation:
```php
WHERE A.post_type = 'shop_order'
AND   A.post_date >= '{$this->dateFrom}'
AND   A.post_date <= '{$this->dateTo}'
```
**Fixed (3.11.3), lines 421–426:** parameterized via `$wpdb->prepare()`:
```php
$query .= $this->db->prepare( "
    AND A.post_date >= %s
    AND A.post_date <= %s
", $this->dateFrom, $this->dateTo );
```
The `LIMIT/OFFSET` clauses and the `countProducts()`/`getBasicProductDataQuery()`
queries were also migrated to `$wpdb->prepare()` (lines 432–439, 668, 689),
removing the remaining `{$limit}`/`{$offset}` interpolations (those were int-cast
already, but are now parameterized too).

### 5. Error-message sanitization
3.11.3 adds `ExportController::sanitize_error_message()` (lines 312–334) that
strips control characters, truncates, and `esc_html()`s every exception message
echoed in `buildResponse()`, preventing XSS/log-injection via reflected error
text (orthogonal to the SQLi fix).

## What invariant the fix relies on

The fix assumes that **all** code paths that reach the
`getExportContactsQuery()` SQL sink (or any `$wpdb` call with user input) are
gated by `SecureHelper::validate_ajax_nonce()` **and** a `manage_options` check,
and that the only attacker-controlled field interpolated into SQL (`dateFrom`)
is now both validated (date format) and parameterized (`%s`). The authorization
fix is centralized in `validate_ajax_nonce()` and additionally re-enforced
per-handler, so it does not depend on every caller remembering to die.

## What code paths the fix explicitly covers

All five export AJAX actions registered in `ExportController::registerActions()`
(3.11.2 line 62–69):
- `salesmanago_export_count_contacts` → `countContacts()`  (parent repro path)
- `salesmanago_export_contacts`       → `exportContacts()` (variant tested here)
- `salesmanago_export_count_events`   → `countEvents()`
- `salesmanago_export_events`         → `exportEvents()`
- `salesmanago_export_products`       → `exportProducts()`

Each now: dies 403 for non-admins (validate_ajax_nonce + per-handler gate),
validates `dateFrom`/`dateTo` in `parseArgs()`, and the contacts query is
`$wpdb->prepare()`-d. The events path (`getEventsData`) never used raw SQL
interpolation (it routes through WooCommerce's `wc_get_orders`, which is itself
parameterized), and the products path (`getBasicProductDataQuery`) only
interpolated int-cast `LIMIT`/`OFFSET` values, now also `prepare()`-d.

## What the fix does NOT cover (gaps evaluated)

I evaluated every other entry point that could reach a SQL sink with
attacker-controlled input:

1. **Other `wp_ajax_*` actions in `Admin.php`** — `salesmanago_refresh_owners`,
   `salesmanago_refresh_catalogs`, `salesmanago_generate_swjs`, and
   `wp_ajax_trash_post` (→ `delete_product`). These call the SALESmanago REST
   API / `wc_get_product()` / `ProductCatalogController->upsertProduct()`, not
   `$wpdb` with raw user-input interpolation. Not SQLi sinks. The
   `wp_ajax_trash_post` registration hijacks a core WP action name but only
   triggers a product upsert to the SALESmanago API; no SQL injection.

2. **REST API endpoints** — `salesmanago/v2/callbackApiV3` (POST,
   `permission_callback => __return_true` but gated by an `sm_token`
   `validate_callback` compared with `hash_equals`) and `salesmanago/v1/cart`
   (GET, unauthenticated, `$_GET['c']` → `WcCartRecoveryModel::recoverCart()`
   which casts every deserialized field to `(int)` before use). Neither reaches
   a raw-SQL sink with attacker-controlled string input. Not SQLi variants.

3. **`CronController::handle_cron_request()`** — gated by a stored secret token
   compared with `hash_equals` (constant-time, not injectable); the user input
   is only the token. Not a SQLi sink.

4. **Other `$wpdb` call sites** — `AdminModel` (all already use `$wpdb->prepare`
   in both versions), `ProductCatalogModel::getAttributesFromDb()` /
   `getCustomAttributesFromDb()` (constant queries, no user input),
   `AdminModel::getAttributes()` (constant query). None interpolate
   attacker-controlled input.

5. **`dateTo` field** — `dateTo` is passed through `date('Y-m-d',
   strtotime($data->dateTo)+86400)` in both versions; `strtotime()` collapses
   injected SQL to a timestamp (or `false`→`1970-01-01`), so `dateTo` was never
   injectable. Only `dateFrom` was the injectable field (assigned raw).

**Conclusion on coverage:** The 3.11.3 fix is **complete** for the SQL-injection
class. The only sink that interpolated attacker-controlled input
(`getExportContactsQuery()` → `dateFrom`) is parameterized, the input is
validated, and **every** handler that reaches it is now authorization-gated for
non-admins. No unpatched SQLi sink or unauthenticated SQLi path was found.

## Behavior before vs after the fix

| Aspect | 3.11.2 (vulnerable) | 3.11.3 (fixed) |
|---|---|---|
| Subscriber reaches export AJAX query | yes (validate_ajax_nonce returns false, no die) | no — `wp_send_json_error(403)` dies + per-handler `manage_options` gate |
| `dateFrom` sanitization | none (raw `$data->dateFrom`) | `sanitize_text_field` + `validate_date_format` |
| `getExportContactsQuery` SQL | string interpolation `'{dateFrom}'` | `$wpdb->prepare("... %s", $dateFrom)` |
| `exportContacts` action name passed to nonce check | `'salesmanago_count_contacts'` (wrong) | `'salesmanago_export_contacts'` (correct) |
| Error messages echoed | raw `$e->getMessage()` | `sanitize_error_message()` (esc_html + trim) |

## Threat-model scope (target's stated position)

SALESmanago & Leadoo is a WordPress plugin with no published `SECURITY.md`. The
WPScan advisory frames the issue as authenticated SQL injection (CVSS 7.7,
"Subscriber+"). The trust boundary is the **network-reachable
`/wp-admin/admin-ajax.php` endpoint** with an authenticated low-privilege
session — the attacker crosses from "logged-in Subscriber" to "arbitrary SQL
against the site database". The variant tested here crosses the **same** trust
boundary (authenticated Subscriber over HTTP to the same admin-ajax endpoint);
it does not introduce a new trust boundary (e.g. unauthenticated), and none of
the unauthenticated surfaces (REST cart, cron) reach the SQL sink.