fix: SQL injection in bulk format change, block editor preview, and PHP 8 deprecations (10.47)

[iftakharul-islam]

Follow-ups from the 10.46 security release. Tracked in weDevsOfficial/plugin-internal-tasks#5.

Security — SQL injection in format_change()

classes/class-s2-admin.php:1207

$format came from $_POST['format'] filtered only by sanitize_text_field(), which strips tags but does not escape SQL quotes, and was then interpolated straight into an UPDATE.

Confirmed exploitable before the fix — this payload executed with no DB error and wrote attacker-controlled SQL:

UPDATE wp_usermeta SET meta_value='html', meta_value='PWNED_BY_SQLI' WHERE meta_key='s2_format' AND user_id IN ('1')

After the fix the same payload is inert, stored as a literal string:

UPDATE wp_usermeta SET meta_value = 'html\', meta_value=\'PWNED_BY_SQLI' WHERE meta_key = 's2_format' AND user_id IN (1)

Requires manage_options and passes a nonce, so this is privilege escalation from a compromised or lower-trust admin rather than an unauthenticated hole — but it is a real injection. Worth noting the same method already protected $useremails and $ids via prepare_in_data(); only $format skipped it, and a phpcs:ignore on the line hid it from tooling.

Also added an early return when no user IDs match, which fixes a pre-existing IN () syntax error on that path.

Correctness — block editor preview

classes/class-s2-block-editor.php:129

preview() read $this->subscribe2_options['email_freq'], but S2_Block_Editor declares no properties, so this was always null. 'never' !== null is always true, meaning preview always took the digest branch and never reached publish(). It also cascaded into Undefined array key "never" at class-s2-core.php:2070.

Now uses the $mysubscribe2 global that the method already declared but never used — matching setting() twenty lines below.

Robustness — confirmation links

classes/class-s2-frontend.php read $_GET['s2'] unguarded and unsanitised in two places, emitting warnings on malformed or absent codes. Now isset-guarded and sanitised. The wp_hash() verification that actually protects the confirm/unsubscribe flow is unchanged.

PHP 8 compatibility

• Declared $script_debug on S2_Ajax, and $email / $ip / $message on S2_Core (used 11× in core and 9× in ShortcodeTrait, declared nowhere). Dynamic property creation is deprecated in 8.2 and fatal in PHP 9.
• Defaulted the substitute() properties to '', post_count to 0, and returned '' instead of null from get_tracking_link() — clearing the stripslashes() / str_replace() null deprecations on 8.1+.
• Corrected post_cat_names / post_tag_names docblocks from array|null to string; both are assigned via implode() and consumed as strings.

Testing

Verified on a live install (WP 7.0.2 / PHP 8.2):

• SQLi — re-ran the exact exploit that succeeded pre-fix; now escaped and inert. Normal format changes still write correctly, and the empty-ID path no longer errors.
• Deprecations — substitute() on a fresh object went from 9 deprecation notices to 0. new S2_Ajax() no longer warns.
• No regressions — XSS fix from 10.46 still holds (DOM parser confirms zero on* attributes); subscribe flow returns the confirmation message and writes active=0; all four admin screens load with no fatals; confirmation links handle absent, empty, junk, and XSS-shaped s2 values without warnings.
• php -l clean across every file in the plugin. debug.log stayed empty throughout.

All test data removed; subscribers and user meta restored to their original values.

Not included

The nojs shortcode argument deprecation from issue #5 is intentionally left out — it is public API used by the widget and Gutenberg block, so it deserves a planned deprecation cycle rather than being bundled into a fix release.

[iftakharul-islam]

Added: broken access control fix (closes #25)

Investigated the open issues on this repo. #25 is real and I reproduced it, so the fix is now in this PR.

#25 — Broken Access Control (Patchstack / chokri hammedi)

All three s2/v1 REST routes were gated on edit_posts, which every role down to Contributor holds. Verified by logging in as a real Contributor:

Endpoint	Contributor could
/resend/{id}	Mass-mail the live subscriber list on a post they don't own
/preview/{id}	Trigger sends for any post id
/settings/{setting}	Read plugin config — sender, tracking, subject templates

The resend one is the serious half: it calls publish($post) with no preview argument, so it targets real subscribers rather than the caller. With two active subscribers I confirmed a Contributor mailed both of them.

Fixed:

• resend → s2_capability('send') (defaults to manage_options, matching the Send Email screen) and edit_post on the target
• preview → edit_post on the specific post, not generic edit_posts
• settings → s2_capability('settings')

Verified after the fix: Contributor, Author, and Editor all get 403 rest_forbidden with zero mail sent; administrator retains full access on all three routes. All testing used a pre_wp_mail filter so no real email left the box.

Also picked up $preview_email — another undeclared dynamic property that only surfaces on the preview path (PHP 8.2 deprecation, fatal in 9), and corrected the settings route args key from 'id' to 'setting', which was validating a parameter that route never receives.

Other open issues — no code change needed

• Support WordPress 7.0 compatibility #29 (WP 7.0) — updated Tested up to: 7.0. Verified against WordPress 7.0.2 / PHP 8.2: all four admin screens, subscribe flow, and block editor integration work, and the deprecation cleanup in this PR covers the "fix PHP notices" task.
• [Bug] Send Email - Same file is already selected + Uploading Plugin from wp.org generates PHP notice #17 — both parts already fixed in current code. uninstall.php now initialises $s2_mu and isset()-guards $wp_version; I ran it and got no notices. The duplicate-file-selection bug is also gone — add_file_upload() creates empty inputs.
• [Enhancement] Add More Files - Remove option  #16 — already implemented. Each added file row gets a dashicons-no-alt remove control.

Suggest closing #17 and #16 as already-resolved, and #25 when this merges.