Fix/issue WOOTAX-46 - Prevent unauthenticated downloads of tax rate backup CSV files#2932
Fix/issue WOOTAX-46 - Prevent unauthenticated downloads of tax rate backup CSV files#2932
Conversation
bartech
left a comment
There was a problem hiding this comment.
Suggested Changes
Suggestions to consider for improving the security hardening and robustness of this PR. Each section shows the proposed change as a diff and explains the reasoning.
1. Use woocommerce_uploads/taxes instead of taxjar-backups
Files: class-wc-connect-functions.php (lines 265, 333), class-wc-connect-help-view.php (line 386)
- $backup_dir = $upload_dir['basedir'] . '/taxjar-backups';
+ $backup_dir = $upload_dir['basedir'] . '/woocommerce_uploads/taxes';- $file_path = $upload_dir['basedir'] . '/taxjar-backups/' . $filename;
+ $file_path = $upload_dir['basedir'] . '/woocommerce_uploads/taxes/' . $filename;Why: WooCommerce already protects wp-content/uploads/woocommerce_uploads/ with its own .htaccess on most installs. Storing tax backups inside this directory means the files inherit that parent-level protection automatically — even if the child .htaccess is somehow deleted, the parent still blocks direct HTTP access. A custom taxjar-backups directory at the uploads root has no such inherited protection.
2. Re-create protection files when .htaccess is missing
File: class-wc-connect-functions.php — backup_existing_tax_rates()
if ( ! file_exists( $backup_dir ) ) {
if ( ! wp_mkdir_p( $backup_dir ) ) {
$backup_dir = $upload_dir['basedir'];
} else {
self::protect_backup_directory( $backup_dir );
}
+ } elseif ( ! file_exists( $backup_dir . '/.htaccess' ) ) {
+ // Re-create protection files if they were removed.
+ self::protect_backup_directory( $backup_dir );
}Why: Currently protect_backup_directory() is only called when the directory is first created. If the directory already exists but its .htaccess was deleted — by an admin cleaning up, a hosting migration, or a prior run that crashed mid-way — the files will remain unprotected indefinitely. This elseif branch detects the missing .htaccess and re-creates all protection files on the next backup.
3. Add index.php alongside index.html in protect_backup_directory()
File: class-wc-connect-functions.php — protect_backup_directory()
$files = array(
array(
'base' => $dir,
'file' => '.htaccess',
'content' => 'DirectoryIndex index.php index.html' . PHP_EOL . 'deny from all',
),
array(
'base' => $dir,
'file' => 'index.html',
'content' => '',
),
+ array(
+ 'base' => $dir,
+ 'file' => 'index.php',
+ 'content' => '<?php' . PHP_EOL . '// Silence is golden.',
+ ),
);Why: The .htaccess DirectoryIndex directive already references both index.php and index.html, but only index.html is created. On servers that ignore .htaccess (Nginx, or Apache with AllowOverride None), directory listing prevention falls back to whether a default index file exists. Most default web server configs look for index.php first. Adding it matches WooCommerce core's own wc_create_files() pattern and provides an extra layer against directory listing.
Also replaces raw file_put_contents() with fopen()/fwrite()/fclose() using @ error suppression. This is more robust on restricted filesystems — fopen() with @ returns false silently instead of emitting a PHP warning that could break AJAX responses.
4. Obfuscate filenames with wp_hash() suffix
File: class-wc-connect-functions.php — backup_existing_tax_rates()
- $backed_up = file_put_contents(
- $backup_dir . '/taxjar-wc_tax_rates-' . date( 'Y-m-d' ) . '-' . time() . '.csv',
- $csv
- );
+ $base_name = 'taxjar-wc_tax_rates-' . gmdate( 'Y-m-d' ) . '-' . time();
+ $hash_suffix = wp_hash( $base_name );
+ $backed_up = file_put_contents(
+ $backup_dir . '/' . $base_name . '-' . $hash_suffix . '.csv',
+ $csv
+ );Why: Two improvements:
date()→gmdate():date()depends on PHP's configured timezone, which can vary between servers and produce inconsistent filenames.gmdate()always uses UTC.wp_hash()suffix: Even inside a protected directory, the filenames are predictable (taxjar-wc_tax_rates-2026-03-03-1709472000.csv). On Nginx — which ignores.htaccessentirely — anyone who guesses the URL can download the file. Appending a 32-characterwp_hash()suffix (keyed to the site's salt) makes brute-forcing infeasible. This is the same pattern WooCommerce uses for log files.
5. Hash legacy filenames during migration
File: class-wc-connect-functions.php — get_backed_up_tax_rate_files()
- $dest = $backup_dir . '/' . basename( $old_file );
+ $old_basename = pathinfo( $old_file, PATHINFO_FILENAME );
+
+ // Add wp_hash() suffix if the file doesn't already have one (32-char hex at the end).
+ if ( ! preg_match( '/-[0-9a-f]{32}$/', $old_basename ) ) {
+ $hash_suffix = wp_hash( $old_basename );
+ $new_filename = $old_basename . '-' . $hash_suffix . '.csv';
+ } else {
+ $new_filename = basename( $old_file );
+ }
+
+ $dest = $backup_dir . '/' . $new_filename;
if ( ! file_exists( $dest ) ) {
- rename( $old_file, $dest );
+ rename( $old_file, $dest );
} else {
- unlink( $old_file );
+ wp_delete_file( $old_file );
}Why: Migrating old files without adding a hash suffix defeats the purpose of filename obfuscation — the old predictable names would persist in the new directory. This checks whether the file already has a 32-char hex suffix and only appends one if missing, so files are never double-hashed.
Also replaces unlink() with wp_delete_file(). This uses the WordPress API which fires the wp_delete_file filter hook, allowing security plugins to log the deletion or perform cleanup.
6. Gate migration behind an option flag
File: class-wc-connect-functions.php — get_backed_up_tax_rate_files()
// Attempt to migrate legacy files from the public uploads root into the protected directory.
- $old_files = glob( $upload_dir['basedir'] . '/taxjar-wc_tax_rates-*.csv' );
-
- if ( ! empty( $old_files ) ) {
+ if ( ! get_option( 'wcs_tax_backup_files_migrated' ) ) {
+ $old_files = glob( $upload_dir['basedir'] . '/taxjar-wc_tax_rates-*.csv' );
+
+ if ( ! empty( $old_files ) ) {
// ... migration logic ...
+ }
+
+ // Mark migration complete only when no old files remain, so it retries if rename() failed.
+ if ( empty( glob( $upload_dir['basedir'] . '/taxjar-wc_tax_rates-*.csv' ) ) ) {
+ update_option( 'wcs_tax_backup_files_migrated', true, false );
+ }
}Why: get_backed_up_tax_rate_files() is called from get_form_data() on every WooCommerce Status page load. Without gating, the migration glob() and rename loop execute every time — even when there are no legacy files left to migrate. On most sites post-migration, this is a wasted glob() call scanning the uploads root on every admin page view.
The option flag is only set when the post-migration glob() confirms no old files remain. This means:
- If
rename()failed for some files, the flag stays unset and migration retries on the next page load. - The third
falseargument toupdate_option()disables autoloading, since this option is only checked on the status page, not on every request.
7. Tighten the filename validation regex
File: class-wc-connect-help-view.php — handle_tax_backup_download()
- // Validate filename matches expected pattern.
- if ( ! preg_match( '/^taxjar-wc_tax_rates-\d{2,4}-\d{2,4}-\d{2,4}-\d+\.csv$/', $filename ) ) {
+ // Validate filename matches expected pattern: YYYY-MM-DD or legacy MM-DD-YYYY, with optional hash suffix.
+ if ( ! preg_match( '/^taxjar-wc_tax_rates-(\d{4}-\d{2}-\d{2}|\d{2}-\d{2}-\d{4})-\d{1,10}(-[0-9a-f]{32})?\.csv$/', $filename ) ) {Why: The original regex \d{2,4}-\d{2,4}-\d{2,4} is too permissive — it would match nonsensical patterns like 99-9999-99 or 9999-99-9999. The replacement uses an alternation group (\d{4}-\d{2}-\d{2}|\d{2}-\d{2}-\d{4}) that only accepts ISO dates (YYYY-MM-DD, used by new backups) or the legacy US format (MM-DD-YYYY, used by old TaxJar plugin).
Additionally:
\d{1,10}caps the Unix timestamp portion, preventing excessive backtracking (minor ReDoS protection).(-[0-9a-f]{32})?is an optional group for the newwp_hash()suffix, so the regex accepts both old (unhashed) and new (hashed) filenames.
8. Fix whitespace alignment in get_form_data()
File: class-wc-connect-help-view.php — get_form_data()
- 'health_items' => $this->get_health_items(),
- 'services' => $this->get_services_items(),
- 'logging_enabled' => $this->logger->is_logging_enabled(),
- 'debug_enabled' => $this->logger->is_debug_enabled(),
- 'logs' => array(
+ 'health_items' => $this->get_health_items(),
+ 'services' => $this->get_services_items(),
+ 'logging_enabled' => $this->logger->is_logging_enabled(),
+ 'debug_enabled' => $this->logger->is_debug_enabled(),
+ 'logs' => array(Why: Cosmetic fix. The => operators were over-padded to align with 'tax_backup_download_url' key from a previous iteration. This normalizes alignment to the current longest key, following WordPress coding standards for associative arrays.
9. Add nocache_headers() and flush output buffers before streaming
File: class-wc-connect-help-view.php — handle_tax_backup_download()
+ nocache_headers();
header( 'Content-Type: text/csv' );
header( 'Content-Disposition: attachment; filename="' . $filename . '"' );
header( 'Content-Length: ' . filesize( $file_path ) );
+
+ // Clean any output buffers to prevent corrupted downloads.
+ while ( ob_get_level() > 0 ) {
+ ob_end_clean();
+ }
+
readfile( $file_path );
exit;Why: Two additions:
-
nocache_headers(): Prevents browsers and proxy caches from storing the authenticated download response. Without this, a shared proxy could serve a cached copy of the CSV to a different user, or the browser cache could serve a stale file after tax rates have changed. -
ob_end_clean()loop: WordPress core, WooCommerce, and other plugins often have output buffers active during AJAX handling. If any buffered content (PHP notices, debug output, plugin HTML) exists whenreadfile()executes, that content is prepended to the CSV data. This causes two problems: (1) theContent-Lengthheader becomes incorrect because it only counts the file's bytes, not the buffered output, and (2) the downloaded file is corrupted with non-CSV data at the beginning. This pattern is used by WooCommerce core inWC_Download_Handler::download_file_force().
* Relocate tax backups to protected directory and harden file security Move backup storage from taxjar-backups to woocommerce_uploads/taxes, add wp_hash() suffix to filenames to prevent URL guessing, and improve directory protection using the WC ReportCSVExporter pattern. Migration logic now uses an idempotency flag (wcs_tax_backup_files_migrated) and wp_delete_file() for safer cleanup of legacy files. * Update download handler for new backup directory and add response hardening Point file download path to woocommerce_uploads/taxes, tighten the filename regex to accept hash-suffixed names and both date formats, and add nocache_headers() plus output buffer cleanup for reliable CSV downloads. * fix the marking migration logic --------- Co-authored-by: Iyut <iyut85@yahoo.com>
|
@bartech , thanks a lot for helping hardening the security and I've merged your changes to this branch. When you are free, please kindly review this PR again. |
Description
Tax rate backup CSV files were stored directly in the public
wp-content/uploads/directory with predictable filenames (e.g.taxjar-wc_tax_rates-04-04-2025-1743750602.csv). Anyone with the URL could download them without authentication.This PR prevents unauthorized downloads by:
wp-content/uploads/taxjar-backups/) with.htaccess(deny from all) and anindex.phpfile to block direct HTTP access.wcs_download_tax_backup) that verifies a nonce and checks for themanage_woocommercecapability before streaming the file.Changes
classes/class-wc-connect-functions.phpbackup_existing_tax_rates(): Saves CSVs touploads/taxjar-backups/and creates protection files on first use.protect_backup_directory()(new): Creates.htaccessandindex.phpin the backup directory.get_backed_up_tax_rate_files(): Moves any legacy files from the public uploads root into the protected directory, then returns a flat array of filenames.classes/class-wc-connect-help-view.phpwp_ajax_wcs_download_tax_backupaction.get_form_data(): Passes a nonce-protected AJAX download URL to the frontend.handle_tax_backup_download()(new): AJAX handler that verifies nonce + capability, validates the filename against a strict regex, and streams the file.client/apps/plugin-status/view.jsRelated issue(s)
Closes WOOTAX-46
Steps to reproduce & screenshots/GIFs
Before (vulnerable):
After (fixed):
-1/ unauthorized)./wp-content/uploads/taxjar-wc_tax_rates-*.csv) returns 404 because the file has been moved to the protected directory.Checklist
changelog.txtentry addedreadme.txtentry added