Skip to content

Commit 122d741

Browse files
authored
Security hardening: authorization, input validation, and logging (#2923)
* scope dynamic records map endpoints to caller's viewable records - authorize and scope cluster_geojson, get_grid_totals, get_list_by_grid_id and points_geojson callbacks - personal map limits results to records shared with the current user; records map requires all-access or project metrics permission * fix sql injection in date_range_activity user_select filter cast the user id to an integer before building the meta_value clause so a crafted value.ID can no longer break out of the query * harden contact receive-transfer against object injection - validate the transfer token before inserting or processing meta, rejecting forged or missing tokens - decode postmeta values with allowed_classes disabled so serialized objects can no longer be instantiated * require a real user for get_settings instead of a dead guard wp_get_current_user() always returns an object, so check ->exists() so the settings endpoint fails closed for logged-out requests * stop logging magic-link request params to the php error log remove leftover debug error_log calls in the dt-home endpoint that wrote the magic-link public_key to the log on every request * stop logging the cleartext password on password change remove the dt_write_log call that wrote the new password to debug.log * escape stored values when rendering revert and name dialogs render activity revert values, merge target name, and delete-filter name as text instead of html to prevent stored xss * require update permission for activity revert and post merge both operations write to records but were gated on can_view; check can_update so a view-only user cannot revert or merge records * throttle brute-force attempts on the jwt token endpoint cap repeated failed logins per client ip on /jwt-auth/v1/token using transients; scoped to the token endpoint and disableable via filter * protect the assigned user's share from removal by collaborators the prior guard was unreachable; a user with only a share could remove the assigned user's share and lock them out. allow self-removal, and restrict removing the assignee's share to that user or update_any holders * validate upload content type and prevent inline-executable files detect the real mime type from the file bytes instead of trusting the client, and store anything that is not a raster image as octet-stream * use constant-time comparison for site-link transfer tokens replace loose == checks with hash_equals so token matching is constant-time and not subject to numeric string type juggling * validate plugin-install download url against ssrf reject non-http(s) schemes and hosts resolving to private, loopback, link-local or reserved ranges, and fetch via the http api * keep peoplegroups locale search inside the permission group the locale clause was appended after the search parenthesis, so its OR escaped the share gate; move it inside so access filters still apply * explain why the assigned user cannot be unshared state the reason (the user is assigned to the record) and make the message translatable instead of a generic permission error * phpcs
1 parent c2ee7bd commit 122d741

17 files changed

Lines changed: 242 additions & 55 deletions

dt-apps/dt-home/magic-link-home-app.php

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -491,12 +491,7 @@ public function add_endpoints() {
491491
public function endpoint_get( WP_REST_Request $request ) {
492492
$params = $request->get_params();
493493

494-
// Debug logging
495-
error_log( 'DT Home Screen REST endpoint called' );
496-
error_log( 'Request params: ' . print_r( $params, true ) );
497-
498494
if ( ! isset( $params['parts'], $params['action'] ) ) {
499-
error_log( 'Missing parameters - parts: ' . ( isset( $params['parts'] ) ? 'yes' : 'no' ) . ', action: ' . ( isset( $params['action'] ) ? 'yes' : 'no' ) );
500495
return new WP_Error( __METHOD__, 'Missing parameters', [ 'status' => 400 ] );
501496
}
502497

@@ -511,8 +506,6 @@ public function endpoint_get( WP_REST_Request $request ) {
511506
$apps_manager = DT_Home_Apps::instance();
512507
$apps = $apps_manager->get_apps_for_user( $user_id );
513508

514-
error_log( 'Apps found: ' . count( $apps ) );
515-
516509
return [
517510
'success' => true,
518511
'apps' => $apps,
@@ -524,8 +517,6 @@ public function endpoint_get( WP_REST_Request $request ) {
524517
$training_manager = DT_Home_Training::instance();
525518
$training_videos = $training_manager->get_videos_for_frontend();
526519

527-
error_log( 'Training videos found: ' . count( $training_videos ) );
528-
529520
return [
530521
'success' => true,
531522
'training_videos' => $training_videos,
@@ -541,9 +532,6 @@ public function endpoint_get( WP_REST_Request $request ) {
541532
$apps = $apps_manager->get_apps_for_user( $user_id );
542533
$training_videos = $training_manager->get_videos_for_frontend();
543534

544-
error_log( 'Apps found: ' . count( $apps ) );
545-
error_log( 'Training videos found: ' . count( $training_videos ) );
546-
547535
return [
548536
'success' => true,
549537
'user_id' => $user_id,

dt-assets/js/comments.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,9 +731,9 @@ jQuery(document).ready(function ($) {
731731
);
732732
}
733733

734-
$('.revert-field').html(field || a.meta_key);
735-
$('.revert-current-value').html(a.meta_value);
736-
$('.revert-old-value').html(a.old_value || 0);
734+
$('.revert-field').text(field || a.meta_key);
735+
$('.revert-current-value').text(a.meta_value);
736+
$('.revert-old-value').text(a.old_value || 0);
737737
})
738738
.catch((err) => {
739739
console.error(err);

dt-assets/js/details.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ jQuery(document).ready(function ($) {
11031103
onClick: function (node, a, item) {
11041104
$('.confirm-merge-with-post').show();
11051105
$('#confirm-merge-with-post-id').val(item.ID);
1106-
$('#name-of-post-to-merge').html(item.name);
1106+
$('#name-of-post-to-merge').text(item.name);
11071107
},
11081108
onResult: function (node, query, result, resultCount) {
11091109
let text = window.TYPEAHEADS.typeaheadHelpText(

dt-assets/js/modular-list.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@
667667
<img style="padding: 0 4px" src="${window.wpApiShare.template_dir}/dt-assets/images/trash.svg">
668668
</span>`);
669669
delete_filter.on('click', function () {
670-
$(`.delete-filter-name`).html(filter.name);
670+
$(`.delete-filter-name`).text(filter.name);
671671
$('#delete-filter-modal').foundation('open');
672672
filter_to_delete = filter.ID;
673673
});

dt-contacts/contacts-transfer.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,11 @@ public static function receive_transferred_contact( $params ) {
536536
$lagging_meta_input = [];
537537
$lagging_location_meta_input = [];
538538
$errors = new WP_Error();
539-
$site_link_post_id = Site_Link_System::get_post_id_by_site_key( Site_Link_System::decrypt_transfer_token( $params['transfer_token'] ) );
539+
$site_link_post_id = empty( $params['transfer_token'] ) ? false : Site_Link_System::get_post_id_by_site_key( Site_Link_System::decrypt_transfer_token( $params['transfer_token'] ) );
540+
if ( empty( $site_link_post_id ) ) {
541+
$errors->add( 'transfer_token_invalid', 'Invalid or missing transfer token' );
542+
return $errors;
543+
}
540544
$field_settings = DT_Posts::get_post_field_settings( $post_args['post_type'] );
541545

542546
/**
@@ -554,7 +558,7 @@ public static function receive_transferred_contact( $params ) {
554558
if ( $key === 'type' && $value[0] === 'media' ) {
555559
$value[0] = 'access';
556560
}
557-
$meta_input[ $key ] = maybe_unserialize( $value[0] );
561+
$meta_input[ $key ] = is_serialized( $value[0] ) ? unserialize( $value[0], [ 'allowed_classes' => false ] ) : $value[0];
558562
}
559563
}
560564
$post_args['meta_input'] = $meta_input;

dt-contacts/duplicates-merging.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,11 @@ public static function merge_posts( $post_type, $primary_post_id, $archiving_pos
485485
return $archiving_post;
486486
}
487487

488+
// A merge mutates both records, so the caller must be able to update both.
489+
if ( !DT_Posts::can_update( $post_type, $primary_post_id ) || !DT_Posts::can_update( $post_type, $archiving_post_id ) ) {
490+
return new WP_Error( __METHOD__, 'No permissions to merge these records', [ 'status' => 403 ] );
491+
}
492+
488493
// Ignore specified fields
489494
$ignored_fields = [
490495
'post_date'

dt-core/admin/admin-settings-endpoints.php

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -787,20 +787,37 @@ public static function edit_translations( WP_REST_Request $request ) {
787787
public function plugin_install( WP_REST_Request $request ) {
788788
require_once( ABSPATH . 'wp-admin/includes/file.php' );
789789
$params = $request->get_params();
790-
$download_url = sanitize_text_field( wp_unslash( $params['download_url'] ) );
790+
$download_url = sanitize_text_field( wp_unslash( $params['download_url'] ?? '' ) );
791+
792+
// Only fetch validated, externally-routable http(s) URLs. wp_http_validate_url() rejects
793+
// loopback, private and link-local addresses, closing server-side request forgery.
794+
if ( empty( $download_url )
795+
|| ! wp_http_validate_url( $download_url )
796+
|| ! in_array( wp_parse_url( $download_url, PHP_URL_SCHEME ), [ 'http', 'https' ], true ) ) {
797+
return new WP_Error( 'invalid_download_url', 'Invalid download URL.', [ 'status' => 400 ] );
798+
}
799+
800+
// wp_http_validate_url() does not cover link-local (e.g. 169.254.169.254 cloud metadata)
801+
// or all reserved ranges; reject any host that resolves into a private or reserved network.
802+
$resolved_ip = gethostbyname( (string) wp_parse_url( $download_url, PHP_URL_HOST ) );
803+
if ( filter_var( $resolved_ip, FILTER_VALIDATE_IP )
804+
&& ! filter_var( $resolved_ip, FILTER_VALIDATE_IP, FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE ) ) {
805+
return new WP_Error( 'invalid_download_url', 'Invalid download URL.', [ 'status' => 400 ] );
806+
}
807+
791808
set_time_limit( 0 );
792-
$folder_name = explode( '/', $download_url );
793-
$folder_name = get_home_path() . 'wp-content/plugins/' . $folder_name[4] . '.zip';
794-
if ( $folder_name != '' ) {
795-
//download the zip file to plugins
796-
file_put_contents( $folder_name, file_get_contents( $download_url ) );
797-
// get the absolute path to $file
798-
$folder_name = realpath( $folder_name );
799-
//unzip
800-
WP_Filesystem();
801-
$unzip = unzip_file( $folder_name, realpath( get_home_path() . 'wp-content/plugins/' ) );
802-
//remove the file
803-
unlink( $folder_name );
809+
WP_Filesystem();
810+
811+
// Download to a temp file through the HTTP API rather than reading the URL directly.
812+
$tmp_file = download_url( $download_url );
813+
if ( is_wp_error( $tmp_file ) ) {
814+
return $tmp_file;
815+
}
816+
817+
$unzip = unzip_file( $tmp_file, realpath( get_home_path() . 'wp-content/plugins/' ) );
818+
unlink( $tmp_file );
819+
if ( is_wp_error( $unzip ) ) {
820+
return $unzip;
804821
}
805822
return true;
806823
}

dt-core/admin/site-link-post-type.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,7 +1419,7 @@ public static function decrypt_transfer_token( $transfer_token ) {
14191419
*/
14201420

14211421
if ( isset( $array['token'], $array['token_as_transfer_key'] ) && $array['token_as_transfer_key'] ){
1422-
if ( $array['token'] == $transfer_token ){
1422+
if ( hash_equals( (string) $array['token'], (string) $transfer_token ) ){
14231423
return $key;
14241424
}
14251425
} else {
@@ -1429,9 +1429,9 @@ public static function decrypt_transfer_token( $transfer_token ) {
14291429
$next = gmdate( 'Y-m-dH', strtotime( current_time( 'Y-m-d H:i:s', 1 ) . '+1 hour' ) );
14301430
$next_hour = md5( $key . $next );
14311431

1432-
if ( $current_hour == $transfer_token
1433-
|| $past_hour == $transfer_token
1434-
|| $next_hour == $transfer_token ){
1432+
if ( hash_equals( $current_hour, (string) $transfer_token )
1433+
|| hash_equals( $past_hour, (string) $transfer_token )
1434+
|| hash_equals( $next_hour, (string) $transfer_token ) ){
14351435

14361436
return $key;
14371437
}

dt-core/core-endpoints.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ public function add_api_routes() {
5555
*/
5656
public static function get_settings() {
5757
$user = wp_get_current_user();
58-
if ( !$user ){
59-
return new WP_Error( 'get_settings', 'Something went wrong. Are you a user?', [ 'status' => 400 ] );
58+
if ( !$user->exists() ){
59+
return new WP_Error( 'get_settings', 'Something went wrong. Are you a user?', [ 'status' => 401 ] );
6060
}
6161
$available_translations = dt_get_available_languages();
6262
$post_types = DT_Posts::get_post_types();

dt-core/dt-storage.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,28 @@ public static function upload_file( string $key_prefix = '', array $upload = [],
165165
$tmp = $upload['tmp_name'] ?? '';
166166
$type = $upload['type'] ?? '';
167167

168+
// Derive the real content type from the file's bytes rather than trusting the
169+
// client-supplied value, and never store uploads as inline-executable content:
170+
// anything that is not a known raster image is served as a download.
171+
if ( $tmp && function_exists( 'finfo_open' ) ) {
172+
$finfo = finfo_open( FILEINFO_MIME_TYPE );
173+
if ( $finfo ) {
174+
$detected = finfo_file( $finfo, $tmp );
175+
finfo_close( $finfo );
176+
if ( !empty( $detected ) ) {
177+
$type = $detected;
178+
}
179+
}
180+
}
181+
$safe_inline_types = [ 'image/gif', 'image/jpeg', 'image/png', 'image/webp' ];
182+
$content_type = in_array( strtolower( trim( $type ) ), $safe_inline_types, true ) ? $type : 'application/octet-stream';
183+
168184
try {
169185
$client->putObject([
170186
'Bucket' => $bucket,
171187
'Key' => $key,
172188
'Body' => fopen( $tmp, 'r' ),
173-
'ContentType' => $type
189+
'ContentType' => $content_type
174190
]);
175191

176192
$uploaded_thumbnail_key = null;

0 commit comments

Comments
 (0)