Fix plugin security issues - #2357
Conversation
…late files
Add `if ( ! defined( 'ABSPATH' ) ) { exit; }` to 71 procedural and template
PHP files flagged by Plugin Check (missing_direct_file_access_protection).
The guard is inserted right after each file's docblock (after the plugin
header in index.php). No behavior change: these files are only ever reached
via WordPress includes where ABSPATH is already defined.
Clears all own-code direct-file-access errors (remaining ones are in bundled
third-party libraries under lib/).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The plugin already calls WordPress functions introduced up to 4.9.6 (wp_get_upload_dir 4.5, wp_delete_file 4.2, wp_parse_url 4.4, get_sites 4.6, wp_add_inline_script 4.5, wp_add_privacy_policy_content 4.9.6). The declared minimum of 4.1 was inaccurate. Metadata-only change (plugin header + readme.txt) — no code or behaviour change. Clears all own-code wp_function_not_compatible_with_requires_wp errors (the 2 remaining are utf8_encode/decode in bundled lib/getid3). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Drop the now-restricted term "WordPress" from the plugin display name
("rtMedia for WordPress, BuddyPress and bbPress" -> "rtMedia for BuddyPress
and bbPress"), clearing the trademarked-term name warnings. Display name
only; the slug (buddypress-media) is unchanged and is grandfathered.
- Bump "Tested up to" 6.9 -> 7.0 to match the current WordPress release,
clearing outdated_tested_upto_header.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…issions and validate debug log attachments
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR hardens the rtMedia plugin primarily by addressing WP.org Plugin Check findings and closing several security gaps (authorization/ownership checks, nonce enforcement, and API privacy enforcement).
Changes:
- Added direct-file-access protection (
ABSPATHguards) across many template/procedural PHP entrypoints. - Strengthened authorization checks for uploads (author/context/album), destructive actions (bulk delete, move/merge, comment delete), and admin/importer AJAX endpoints (nonce + capability).
- Improved API privacy controls (view-permission gating for feeds/likes/comments/media details) and hardened DB query construction (operator and ORDER BY allowlisting).
Reviewed changes
Copilot reviewed 109 out of 112 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| templates/upload/uploader.php | Add ABSPATH guard; add PHPCS ignore annotations for legacy variable names |
| templates/upload/comment-media.php | Add ABSPATH guard; add PHPCS ignore annotations for legacy variable names |
| templates/media/media-single.php | Add ABSPATH guard; add PHPCS ignore annotations for legacy symbols/hooks |
| templates/media/media-single-edit.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/media/media-gallery.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/media/media-gallery-item.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/media/godam-integration.php | Add ABSPATH guard; add script versioning; add PHPCS ignore annotations |
| templates/media/album-single-edit.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/media/album-gallery.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/media/album-gallery-item.php | Add ABSPATH guard; add PHPCS ignore annotations |
| templates/main.php | Add ABSPATH guard; add PHPCS ignore annotations for legacy hooks/vars |
| readme.txt | Update “Requires at least” and “Tested up to” metadata |
| index.php | Update “Requires at least”; add ABSPATH guard; annotate legacy function name |
| app/main/templates/privacy-content.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/main/templates/policy-information.php | Add ABSPATH guard |
| app/main/templates/merge-album-modal.php | Add ABSPATH guard |
| app/main/templates/media-upload-terms.php | Add ABSPATH guard |
| app/main/templates/media-pagination.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/main/templates/media-group-edit-screen.php | Add ABSPATH guard |
| app/main/templates/media-group-create-screen.php | Add ABSPATH guard |
| app/main/templates/image-editor-content.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/main/templates/create-album-modal.php | Add ABSPATH guard |
| app/main/templates/admin-pages-content.php | Add ABSPATH guard |
| app/main/routers/query/RTMediaQuery.php | Harden request arg filtering; cast allowed_types; annotate meta/tax query slow-query warnings |
| app/main/routers/RTMediaRouter.php | Remove stale AJAX action registration; add PHPCS ignore annotations |
| app/main/controllers/upload/processors/RTMediaUploadFile.php | Cast allowed_types to array; annotate legacy global var |
| app/main/controllers/upload/RTMediaUploadView.php | Expand PHPCS ignore list for prepared direct DB query |
| app/main/controllers/upload/RTMediaUploadModel.php | Enforce acting author, authorize upload targets, restrict privacy levels; implement album permission checks |
| app/main/controllers/upload/RTMediaUploadEndpoint.php | Re-apply target authorization after filters, before upload execution |
| app/main/controllers/upload/RTMediaUpload.php | Annotate legacy action hook name |
| app/main/controllers/template/rtmedia-functions.php | Add ABSPATH guard; add manage/album permission helpers; add nonce for comment deletion; cast allowed_types |
| app/main/controllers/template/rtmedia-filters.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/main/controllers/template/rtmedia-ajax-actions.php | Add ABSPATH guard; tighten admin definition to list_users/super_admin |
| app/main/controllers/template/rtmedia-actions.php | Add ABSPATH guard; annotate legacy functions/hooks |
| app/main/controllers/template/RTMediaUploadTemplate.php | Annotate legacy hook names |
| app/main/controllers/template/RTMediaTemplate.php | Add ownership checks for move/merge/bulk delete; fix inverted delete-comment guard; add nonce + permission checks for comment delete |
| app/main/controllers/template/RTMediaNav.php | Cast allowed_types to array; annotate legacy hook |
| app/main/controllers/template/RTMediaAJAX.php | Annotate legacy hook names |
| app/main/controllers/shortcodes/RTMediaUploadShortcode.php | Annotate legacy hook name |
| app/main/controllers/shortcodes/RTMediaGalleryShortcode.php | Annotate legacy hook names |
| app/main/controllers/privacy/RTMediaPrivacy.php | Add ownership/gate checks for activity privacy updates; validate allowed privacy levels |
| app/main/controllers/media/RTMediaMeta.php | Annotate slow-query warnings for custom meta table columns |
| app/main/controllers/media/RTMediaMedia.php | Annotate slow-query warnings for custom meta table columns |
| app/main/controllers/media/RTMediaLoginPopup.php | Annotate legacy hook names |
| app/main/controllers/media/RTMediaGroupFeatured.php | Add ABSPATH guard |
| app/main/controllers/media/RTMediaFeatured.php | Add ABSPATH guard; annotate legacy functions |
| app/main/controllers/media/RTMediaComment.php | Annotate legacy hook name |
| app/main/controllers/group/RTMediaGroupExtension.php | Add ABSPATH guard |
| app/main/controllers/api/RTMediaJsonApiFunctions.php | Add explicit privacy gating to API feed/album listing; fix WP_Comment_Query args |
| app/main/controllers/api/RTMediaJsonApi.php | Enforce site registration policy; gate likes/comments/likes-list by media visibility; fix remove_comment authorization; add API privacy helper |
| app/main/controllers/activity/RTMediaBuddyPressActivity.php | Annotate non-WP_Query exclude parameter usage; annotate legacy hooks |
| app/main/RTMediaUploadTerms.php | Add ABSPATH guard; justify load_plugin_textdomain usage |
| app/main/RTMedia.php | Cast allowed_types/default_sizes to array; add PHPCS ignores; justify load_plugin_textdomain; tag direct DB ignores |
| app/importers/templates/media-size-importer.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/importers/templates/activity-upgrade.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/importers/RTMediaMigration.php | Add nonce + manage_options checks for migration AJAX; add PHPCS ignores for direct DB access |
| app/importers/RTMediaMediaSizeImporter.php | Add nonce + capability checks to AJAX actions; pass nonce in notice; allow data-nonce in KSES |
| app/importers/RTMediaActivityUpgrade.php | Add nonce + capability checks for AJAX actions; annotate direct DB queries |
| app/importers/BPMediaImporter.php | Annotate legacy non-prefixed class name |
| app/importers/BPMediaAlbumimporter.php | Add manage_options checks; fix inverted deactivate logic; annotate direct DB and meta_key warnings |
| app/helper/templates/themes-content.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/helper/templates/support-form.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/helper/templates/submit-request.php | Add ABSPATH guard |
| app/helper/templates/service-sector.php | Add ABSPATH guard |
| app/helper/templates/debug-info.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/helper/templates/addon.php | Add ABSPATH guard |
| app/helper/templates/3rd-party-themes-content.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/helper/rtProgress.php | Annotate legacy non-prefixed class name |
| app/helper/rtPluginUpdateChecker.php | Annotate legacy non-prefixed class name |
| app/helper/rtFormInvalidArgumentsException.php | Add ABSPATH guard; annotate legacy non-prefixed class name |
| app/helper/rtForm.php | Add ABSPATH guard; annotate legacy non-prefixed class name |
| app/helper/rtDimensions.php | Annotate legacy non-prefixed class name |
| app/helper/db/rt_plugin_info.php | Add ABSPATH guard |
| app/helper/db/RTDBUpdate.php | Add ABSPATH guard; annotate legacy hook; extend direct DB PHPCS ignores |
| app/helper/db/RTDBModel.php | Add ABSPATH guard; sanitize compare operator and order_by input |
| app/helper/RTMediaUploadException.php | Annotate legacy hook names |
| app/helper/RTMediaSupport.php | Add ABSPATH guard; add capability check; harden attachment path and From header sanitization |
| app/helper/RTMediaSettings.php | Add ABSPATH guard; cast allowed_types/default_sizes to array |
| app/helper/RTMediaModel.php | Use compare-operator allowlist; sanitize order_by for album queries; cast allowed_types |
| app/helper/RTMediaAdminWidget.php | Add ABSPATH guard |
| app/helper/RTMediaAddon.php | Add ABSPATH guard |
| app/assets/js/rtMedia.backbone.js | Send delete-comment nonce with AJAX requests |
| app/assets/admin/js/rtmedia-admin.js | Send nonce when dismissing admin notice |
| app/assets/admin/js/migration.js | Send nonce with migration AJAX requests |
| app/assets/admin/js/importer.js | Send nonce when marking activity upgrade complete |
| app/admin/templates/tmpl-rtm-theme-overlay.php | Add ABSPATH guard |
| app/admin/templates/tmpl-rtm-p-tag.php | Add ABSPATH guard |
| app/admin/templates/tmpl-rtm-msg-div.php | Add ABSPATH guard |
| app/admin/templates/tmpl-rtm-map-mapping-failure.php | Add ABSPATH guard |
| app/admin/templates/tmpl-rtm-image.php | Add ABSPATH guard |
| app/admin/templates/tmpl-rtm-album-favourites-importer.php | Add ABSPATH guard |
| app/admin/templates/settings/sidebar-branding.php | Add ABSPATH guard |
| app/admin/templates/settings/sidebar-addons.php | Add ABSPATH guard |
| app/admin/templates/settings/render-option.php | Add ABSPATH guard |
| app/admin/templates/settings/media-types.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/settings/media-sizes.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/settings/main.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/settings/admin-ui.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/notices/upload-file-types.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/notices/update-template.php | Add ABSPATH guard |
| app/admin/templates/notices/transcoder.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/notices/premium-addon.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/notices/inspirebook-release.php | Add ABSPATH guard |
| app/admin/templates/notices/addon-update.php | Add ABSPATH guard; add PHPCS ignore annotations |
| app/admin/templates/dashboard-widgets/right-now.php | Add ABSPATH guard; add PHPCS ignore annotations for legacy vars and direct DB queries |
| app/admin/RTMediaUploadTermsAdmin.php | Add ABSPATH guard |
| app/admin/RTMediaFormHandler.php | Add ABSPATH guard |
| app/admin/RTMediaAdmin.php | Add ABSPATH guard; remove unused AJAX endpoints; add capability checks |
| .distignore | Exclude additional dev/CI assets from release builds |
Suppressed comments (5)
app/main/controllers/api/RTMediaJsonApiFunctions.php:1
- In this method scope,
$rtmediajsonapiis referenced without being imported (e.g., viaglobal $rtmediajsonapi) or otherwise initialized. As written,isset( $rtmediajsonapi )will be false and the loop willcontinuefor every item, effectively stripping all media from feeds. Fix by explicitly making the request instance available here (e.g.,global $rtmediajsonapi;) or by using a reliably available API object reference (such as passing it in / storing it on$this) so the visibility check can run.
app/helper/db/RTDBModel.php:1 - The ORDER BY builder prepends
{$this->table_name}.to the entire clause returned bysanitize_order_by(). If the order-by contains multiple segments (e.g.col1 desc, col2 asc), the resulting SQL becomesORDER BY table.col1 desc, col2 asc(the second column is unqualified), which can break queries or change semantics when joins introduce ambiguous column names. Fix by qualifying each sanitized column segment with the table name when constructing the ORDER BY clause, or by restrictingsanitize_order_by()to a single segment if multi-column ordering isn't supported.
app/helper/db/RTDBModel.php:1 - The ORDER BY builder prepends
{$this->table_name}.to the entire clause returned bysanitize_order_by(). If the order-by contains multiple segments (e.g.col1 desc, col2 asc), the resulting SQL becomesORDER BY table.col1 desc, col2 asc(the second column is unqualified), which can break queries or change semantics when joins introduce ambiguous column names. Fix by qualifying each sanitized column segment with the table name when constructing the ORDER BY clause, or by restrictingsanitize_order_by()to a single segment if multi-column ordering isn't supported.
app/helper/RTMediaSupport.php:1 $attachmentsis always set to an array containing one element, even when no attachment is allowed/resolved (i.e.,''). Passing an empty string as an attachment path can causewp_mail()to attempt to attach an invalid file and may trigger warnings or unexpected behavior. Fix by only adding$attachment_fileto the attachments array when it is non-empty (or set$attachmentsto an empty array by default).
app/helper/RTMediaUploadException.php:1- Fix typo in user-facing message: change 'Uploade failed due to internal server error.' to 'Upload failed due to internal server error.'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 111 out of 118 changed files in this pull request and generated no new comments.
Files not reviewed (4)
- app/assets/admin/css/admin.css: Generated file
- app/assets/admin/css/rtm-upload-terms.min.css: Generated file
- app/assets/admin/css/widget.css: Generated file
- app/assets/admin/css/widget.min.css: Generated file
Suppressed comments (6)
app/main/controllers/api/RTMediaJsonApiFunctions.php:1
$rtmediajsonapiis referenced without being imported into scope (there’s noglobal $rtmediajsonapi;here, unlikertmedia_api_album_media). In PHP this makesisset( $rtmediajsonapi )always false, so all media rows get filtered out and API feeds will return no media. Fix by addingglobal $rtmediajsonapi;before using it, or refactor this helper to accept the API instance (or a callable) explicitly.
templates/media/media-gallery.php:1- The
hrefattribute won’t output anything becauseesc_url()is called withoutecho. Change to output the escaped URL (e.g.,href=\"<?php echo esc_url( rtmedia_pagination_next_link() ); ?>\") so the “Load More/Next” link works.
app/helper/db/RTDBModel.php:1 - The builder treats all operators the same and always quotes/wraps the RHS as
('value'). ForIS/IS NOTthis will generate invalid/incorrect SQL (e.g.,IS ('NULL')instead ofIS NULL). HandleIS/IS NOTas special cases (allow onlyNULL/NOT NULLwithout quotes), or remove those operators from the allowlist if they aren’t supported by this query builder.
app/main/controllers/api/RTMediaJsonApi.php:1 - Variable shadowing:
$commentis first aWP_Comment, then reassigned to anRTMediaCommentinstance. This makes the code harder to follow and increases the chance of accidental misuse later in the method. Use distinct variable names (e.g.,$wp_commentand$rtmedia_comment).
app/main/controllers/api/RTMediaJsonApi.php:1 - Variable shadowing:
$commentis first aWP_Comment, then reassigned to anRTMediaCommentinstance. This makes the code harder to follow and increases the chance of accidental misuse later in the method. Use distinct variable names (e.g.,$wp_commentand$rtmedia_comment).
app/admin/templates/dashboard-widgets/right-now.php:29 - Cache key mismatch: you read from
wp_cache_get('rt-stats', ...)but write towp_cache_set('stats', ...), so the cache will never be hit and the DB query will run on every page load. Use the same cache key for get/set (either bothrt-statsor bothstats).
$results = wp_cache_get( 'rt-stats', 'rt-dashboard' ); /* phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound -- Legacy public naming retained for backward compatibility; renaming breaks dependent themes/add-ons. */
if ( false === $results ) {
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter, WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound -- Direct query required; safe because table name is trusted.
$results = $wpdb->get_results( $wpdb->prepare( "SELECT media_type, count(id) as count FROM {$rtmedia_model->table_name} WHERE blog_id=%d GROUP BY media_type", get_current_blog_id() ) );
wp_cache_set( 'stats', $results, 'rt-dashboard', HOUR_IN_SECONDS );
}
@the-hercules Can you have a look at this as well ? |
|
Unable to PHPCS or SVG scan one or more files due to error running PHPCS/SVG scanner:
The error may be temporary. If the error persists, please contact a human (commit-ID: 4a694ec). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 111 out of 118 changed files in this pull request and generated no new comments.
Files not reviewed (4)
- app/assets/admin/css/admin.css: Generated file
- app/assets/admin/css/rtm-upload-terms.min.css: Generated file
- app/assets/admin/css/widget.css: Generated file
- app/assets/admin/css/widget.min.css: Generated file
Suppressed comments (3)
app/helper/db/RTDBModel.php:1
- In
RTDBModel::get(), whenvalueis not set you assign the full$colvaluearray (which may include keys likecompare) into$colvalue['value']. This can generate invalid SQL (e.g., leaking the operator into the value list) and break queries. Align this logic withRTMediaModel::get()by using a$tmp_val = isset($colvalue['value']) ? $colvalue['value'] : $colvalue;approach (or stripping non-value keys likecompare) before building$col_val_comapare.
app/helper/db/RTDBModel.php:1 - In
RTDBModel::get(), whenvalueis not set you assign the full$colvaluearray (which may include keys likecompare) into$colvalue['value']. This can generate invalid SQL (e.g., leaking the operator into the value list) and break queries. Align this logic withRTMediaModel::get()by using a$tmp_val = isset($colvalue['value']) ? $colvalue['value'] : $colvalue;approach (or stripping non-value keys likecompare) before building$col_val_comapare.
app/admin/templates/dashboard-widgets/right-now.php:29 - The cache key used for read/write does not match (
wp_cache_get('rt-stats', ...)vswp_cache_set('stats', ...)), so the cache will never be hit and this widget will always run the DB query. Use the same cache key string for both calls (eitherrt-statsorstats).
$results = wp_cache_get( 'rt-stats', 'rt-dashboard' ); /* phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound -- Legacy public naming retained for backward compatibility; renaming breaks dependent themes/add-ons. */
if ( false === $results ) {
// phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching, WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter, WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedVariableFound -- Direct query required; safe because table name is trusted.
$results = $wpdb->get_results( $wpdb->prepare( "SELECT media_type, count(id) as count FROM {$rtmedia_model->table_name} WHERE blog_id=%d GROUP BY media_type", get_current_blog_id() ) );
wp_cache_set( 'stats', $results, 'rt-dashboard', HOUR_IN_SECONDS );
}
Security hardening + Plugin Check remediation (WP.org report, part 2)
Background
Part 1 (already handled on fix/patchstack-security-fixes) patched the 3 explicitly reported bugs: a SQL‑injection via the compare operator, a JSON‑API privacy bypass, and an album‑injection upload bug. This branch is built on top of
fix/patchstack-security-fixes.This issue tracks that follow‑up work. It is split into three phases so we can pick items up one commit at a time.
Working branch: fix/plugin-check-issues
How we verify: every change is checked against the real PCP tool (wp plugin check buddypress-media) run locally, and every edited PHP file is syntax‑checked (php -l).
Legend — Severity: 🔴 High · 🟠 Medium · 🟡 Low. Status: ✅ done · ⬜ to do.
Baseline (what PCP reported)
Full run: 372 errors + 649 warnings = 1021.
PCP also honours inline phpcs:ignore comments, so a chunk of the codebase's warnings are already suppressed.
Phase 1 — Plugin Check: ERRORS ✅ (complete, ready for review)
Goal: clear every error PCP raises in our own code. Result: own‑code errors 87 → 1 (the one remaining is a false positive, see below).
Known remaining (intentionally not fixed):
Phase 2 — Plugin Check: WARNINGS ⬜ (next)
Goal: clean up the remaining warnings in our own code. None of these are exploitable; they're code‑standards/quality signals. Grouped so each becomes one commit.
Phase 3 — Security hardening: issues similar to the reported ones ⬜
These are the "find similar holes" part of the WP request. Plugin Check cannot detect these — they were found by manual/AI audit of the same code paths as the 3 reported bugs, and each was verified by reading the code. They are the highest‑impact items in this issue.
Common theme: a request supplies an object id (media / album / activity / comment) and the handler acts on it without checking the current user owns it — the same class as the reported album‑injection and privacy‑bypass bugs. The fix pattern is consistent: enforce an ownership/capability check in the handler (mirroring the already‑correct rtmedia_delete_uploaded_media), or the existing API privacy helper.
S1 — Cross‑user write/delete (IDOR) — the four confirmed High bugs 🔴
S2 — JSON/Mobile API cross‑user reads (privacy bypass) 🟠 RTMediaJsonApi*.php
Same root cause as the reported API bug: the privacy filter isn't active on the API path, so several endpoints return other users' private data. All fixable with the existing rtmedia_api_current_user_can_view_media() helper.
S3 — Missing capability / nonce on admin & destructive AJAX 🟠🟡
S4 — SQL hardening (latent, defense‑in‑depth) 🟡
S5 — Dead code / minor 🟡