-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🎯 fix: DB related PHPCS security issues #2271
base: develop
Are you sure you want to change the base?
Changes from all commits
c9c11a2
b5e2c1e
dff5003
642a760
2364453
467e91d
21cd074
e1c9938
711dab9
ee721bc
e1637b0
3455369
7629ff0
7b70269
d344058
900c7be
105d7dd
cdb9839
9eb13a2
dcd5f2f
86d81fb
2d57f2e
303eeb5
54dcc2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,10 +38,8 @@ function dokan_save_product( $args ) { | |
if ( absint( $data['product_cat'] ) < 0 ) { | ||
return new WP_Error( 'no-category', __( 'Please select a category', 'dokan-lite' ) ); | ||
} | ||
} else { | ||
if ( ! isset( $data['product_cat'] ) && empty( $data['product_cat'] ) ) { | ||
return new WP_Error( 'no-category', __( 'Please select at least one category', 'dokan-lite' ) ); | ||
} | ||
} elseif ( ! isset( $data['product_cat'] ) && empty( $data['product_cat'] ) ) { | ||
return new WP_Error( 'no-category', __( 'Please select at least one category', 'dokan-lite' ) ); | ||
} | ||
} elseif ( empty( $data['chosen_product_cat'] ) ) { | ||
return new WP_Error( 'no-category', __( 'Please select a category', 'dokan-lite' ) ); | ||
|
@@ -89,10 +87,8 @@ function dokan_save_product( $args ) { | |
if ( ! isset( $data['chosen_product_cat'] ) ) { | ||
if ( Helper::product_category_selection_is_single() ) { | ||
$cat_ids[] = $data['product_cat']; | ||
} else { | ||
if ( ! empty( $data['product_cat'] ) ) { | ||
$cat_ids = array_map( 'absint', (array) $data['product_cat'] ); | ||
} | ||
} elseif ( ! empty( $data['product_cat'] ) ) { | ||
$cat_ids = array_map( 'absint', (array) $data['product_cat'] ); | ||
} | ||
$post_data['categories'] = $cat_ids; | ||
} | ||
|
@@ -189,7 +185,7 @@ function dokan_product_output_variations() { | |
} | ||
} | ||
|
||
$variations_count = absint( $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(ID) FROM $wpdb->posts WHERE post_parent = %d AND post_type = 'product_variation' AND post_status IN ('publish', 'private', 'pending')", $post->ID ) ) ); | ||
$variations_count = absint( $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(ID) FROM $wpdb->posts WHERE post_parent = %d AND post_type = 'product_variation' AND post_status IN ('publish', 'private', 'pending')", $post->ID ) ) ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optimize the retrieval of product variations. The method + $variations_count = wp_cache_get('variations_count_' . $post->ID);
+ if (false === $variations_count) {
+ $variations_count = $wpdb->get_var(...);
+ wp_cache_set('variations_count_' . $post->ID, $variations_count);
+ }
|
||
$variations_per_page = absint( apply_filters( 'woocommerce_admin_meta_boxes_variations_per_page', 15 ) ); | ||
$variations_total_pages = ceil( $variations_count / $variations_per_page ); ?> | ||
<div id="dokan-variable-product-options" class=""> | ||
|
@@ -403,11 +399,10 @@ function dokan_search_seller_products( $term, $user_ids = false, $type = '', $in | |
$query_args[] = $user_ids; | ||
} | ||
} | ||
// phpcs:ignore WordPress.DB.PreparedSQL | ||
// phpcs:disable WordPress.DB.PreparedSQL, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.PreparedSQLPlaceholders.ReplacementsWrongNumber, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$product_ids = $wpdb->get_col( | ||
// phpcs:disable | ||
$wpdb->prepare( " | ||
SELECT DISTINCT posts.ID FROM {$wpdb->posts} posts | ||
$wpdb->prepare( | ||
"SELECT DISTINCT posts.ID FROM {$wpdb->posts} posts | ||
LEFT JOIN {$wpdb->postmeta} postmeta ON posts.ID = postmeta.post_id | ||
$type_join | ||
WHERE ( | ||
|
@@ -421,12 +416,11 @@ function dokan_search_seller_products( $term, $user_ids = false, $type = '', $in | |
AND posts.post_status IN ('" . implode( "','", $post_statuses ) . "') | ||
$type_where | ||
$users_where | ||
ORDER BY posts.post_parent ASC, posts.post_title ASC | ||
", | ||
ORDER BY posts.post_parent ASC, posts.post_title ASC", | ||
$query_args | ||
) | ||
// phpcs:enable | ||
); | ||
// phpcs:enable | ||
|
||
if ( is_numeric( $term ) ) { | ||
$post_id = absint( $term ); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,9 +20,9 @@ class Tracker { | |||||||||||||||||
/** | ||||||||||||||||||
* Class constructor | ||||||||||||||||||
* | ||||||||||||||||||
* @return void | ||||||||||||||||||
* @since 2.8.7 | ||||||||||||||||||
* | ||||||||||||||||||
* @return void | ||||||||||||||||||
*/ | ||||||||||||||||||
public function __construct() { | ||||||||||||||||||
$this->appsero_init_tracker_dokan(); | ||||||||||||||||||
|
@@ -31,9 +31,9 @@ public function __construct() { | |||||||||||||||||
/** | ||||||||||||||||||
* Initialize the plugin tracker | ||||||||||||||||||
* | ||||||||||||||||||
* @return void | ||||||||||||||||||
* @since 2.8.7 | ||||||||||||||||||
* | ||||||||||||||||||
* @return void | ||||||||||||||||||
*/ | ||||||||||||||||||
public function appsero_init_tracker_dokan() { | ||||||||||||||||||
$client = new \Appsero\Client( '559bcc0d-21b4-4b34-8317-3e072badf46d', 'Dokan Multivendor Marketplace', DOKAN_FILE ); | ||||||||||||||||||
|
@@ -65,18 +65,18 @@ function () { | |||||||||||||||||
protected function get_order_count() { | ||||||||||||||||||
global $wpdb; | ||||||||||||||||||
|
||||||||||||||||||
return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); | ||||||||||||||||||
return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use of raw SQL and ignoring PHPCS checks While the comment clarifies why PHPCS checks are disabled, consider using - return (int) $wpdb->get_var( "SELECT count(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ('wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded');" ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
+ $statuses = [ 'wc-completed', 'wc-processing', 'wc-on-hold', 'wc-refunded' ];
+ $placeholders = implode( ', ', array_fill(0, count($statuses), '%s') );
+ $query = $wpdb->prepare(
+ "SELECT COUNT(id) FROM {$wpdb->prefix}dokan_orders WHERE order_status IN ($placeholders)",
+ $statuses
+ );
+ return (int) $wpdb->get_var( $query ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.NoCaching 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Gets custom deactivation reasons | ||||||||||||||||||
* | ||||||||||||||||||
* @since 3.0.15 | ||||||||||||||||||
* | ||||||||||||||||||
* @param string[] $reasons | ||||||||||||||||||
* @param null|\AppSero\Client $client | ||||||||||||||||||
* | ||||||||||||||||||
* @return \array | ||||||||||||||||||
* @since 3.0.15 | ||||||||||||||||||
* | ||||||||||||||||||
*/ | ||||||||||||||||||
public function get_custom_deactivation_reasons( $reasons, $client = null ) { | ||||||||||||||||||
// return if old version of appsero client is loaded, where corresponding hooks provides only one argument | ||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,13 +97,15 @@ public function is_valid_approval_request( $args ) { | |
public function is_valid_cancellation_request( $args ) { | ||
global $wpdb; | ||
|
||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$result = $wpdb->get_row( | ||
$wpdb->prepare( | ||
"select * from {$wpdb->dokan_withdraw} where id = %d and status = %d", | ||
$args['id'], | ||
$this->get_status_code( 'pending' ) | ||
) | ||
); | ||
// phpcs:enable | ||
|
||
if ( ! empty( $result ) ) { | ||
// permission: vendor -> only own && shop_manager | ||
|
@@ -135,6 +137,7 @@ public function update_status( $id, $user_id, $status ) { | |
// 1 -> active | ||
// 2 -> cancelled | ||
|
||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$updated = $wpdb->update( | ||
$wpdb->dokan_withdraw, | ||
[ | ||
|
@@ -152,6 +155,7 @@ public function update_status( $id, $user_id, $status ) { | |
'%d', | ||
] | ||
); | ||
// phpcs:enable | ||
|
||
if ( $updated !== 1 ) { | ||
return new WP_Error( 'dokan_withdraw_unable_to_update', __( 'Could not update withdraw status', 'dokan-lite' ) ); | ||
|
@@ -185,7 +189,7 @@ public function insert_withdraw( $args = [] ) { | |
|
||
$format = [ '%d', '%f', '%s', '%d', '%s', '%s', '%s', '%s' ]; | ||
|
||
$inserted = $wpdb->insert( $wpdb->dokan_withdraw, $data, $format ); | ||
$inserted = $wpdb->insert( $wpdb->dokan_withdraw, $data, $format ); // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Potential Issue with Direct Database Insertion and Lack of Data Sanitization The
Please ensure that all data passed to the database is properly sanitized and validated. Analysis chainCheck for potential issues with direct database insertions. The method Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify data sanitization in `insert_withdraw` method.
# Test: Search for the method usage. Expect: Proper sanitization before data is passed.
rg --type php $'insert_withdraw'
Length of output: 113 Script: #!/bin/bash
# Description: Extract the full implementation of the `insert_withdraw` method to verify data sanitization.
# Searching for the full implementation of the `insert_withdraw` method in the PHP file.
ast-grep --lang php --pattern 'public function insert_withdraw($_) { $$$ }' includes/Withdraw/Manager.php
Length of output: 214 Script: #!/bin/bash
# Description: Extract the full implementation of the `insert_withdraw` method to verify data sanitization.
# Using ripgrep to search for the full implementation of the `insert_withdraw` method.
rg --context 20 'public function insert_withdraw' includes/Withdraw/Manager.php
Length of output: 1407 Script: #!/bin/bash
# Description: Search for all instances where `insert_withdraw` is called to verify data sanitization.
# Using ripgrep to find all invocations of the `insert_withdraw` method.
rg 'insert_withdraw\('
Length of output: 106 |
||
|
||
if ( $inserted !== 1 ) { | ||
return new WP_Error( 'dokan_withdraw_unable_to_insert', __( 'Could not add new withdraw approval request.', 'dokan-lite' ) ); | ||
|
@@ -219,6 +223,7 @@ public function has_pending_request( $user_id ) { | |
|
||
$wpdb->dokan_withdraw = $wpdb->prefix . 'dokan_withdraw'; | ||
|
||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$status = $wpdb->get_results( | ||
$wpdb->prepare( | ||
"SELECT id | ||
|
@@ -227,6 +232,7 @@ public function has_pending_request( $user_id ) { | |
$user_id | ||
) | ||
); | ||
// phpcs:enable | ||
|
||
if ( $status ) { | ||
return true; | ||
|
@@ -256,9 +262,11 @@ public function get_withdraw_requests( $user_id = '', $status = 0, $limit = 10, | |
if ( false === $result ) { | ||
global $wpdb; | ||
if ( empty( $user_id ) ) { | ||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$result = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$wpdb->dokan_withdraw} WHERE status = %d LIMIT %d, %d", $status, $offset, $limit ) ); | ||
} else { | ||
$result = $wpdb->get_results( $wpdb->prepare( "SELECT * FROM {$wpdb->dokan_withdraw} WHERE user_id = %d AND status = %d ORDER BY id DESC LIMIT %d, %d", $user_id, $status, $offset, $limit ) ); | ||
//phpcs:enable | ||
Comment on lines
+265
to
+269
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optimize the retrieval of withdrawal requests. The method + $cache_result = wp_cache_get($cache_key, $cache_group);
+ if (false !== $cache_result) {
+ return $cache_result;
+ }
- $result = $wpdb->get_results(...);
+ wp_cache_set($cache_key, $result, $cache_group);
|
||
} | ||
|
||
$result = array_map( | ||
|
@@ -398,12 +406,14 @@ public function get( $id ) { | |
global $wpdb; | ||
|
||
if ( ! is_array( $id ) ) { | ||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$result = $wpdb->get_row( | ||
$wpdb->prepare( | ||
"select * from {$wpdb->dokan_withdraw} where id = %d", | ||
$id | ||
), ARRAY_A | ||
); | ||
// phpcs:enable | ||
} else { | ||
$attributes = [ | ||
'id' => '%d', | ||
|
@@ -522,6 +532,7 @@ public function get_user_withdraw_summary( $user_id = '' ) { | |
$user_id = dokan_get_current_user_id(); | ||
} | ||
|
||
// phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
$results = $wpdb->get_row( | ||
$wpdb->prepare( | ||
"SELECT | ||
|
@@ -535,6 +546,7 @@ public function get_user_withdraw_summary( $user_id = '' ) { | |
), | ||
ARRAY_A | ||
); | ||
// phpcs:enable | ||
|
||
$summary = [ | ||
'total' => ! empty( $results['total'] ) ? absint( $results['total'] ) : 0, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra empty lines.
Ensure there are no multiple consecutive empty lines within functions.
Committable suggestion
Tools
GitHub Check: Run PHPCS inspection