Steve-Dee-Designs
/
woocommerce
mirror of https://github.com/woocommerce/woocommerce.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #32771 from woocommerce/fix/32754-inactive-downloads 

Protect against inappropriate use of saved download links
This commit is contained in:
Vedanshu Jain 2022-04-27 21:36:37 +05:30 committed by GitHub
parent 9a71334fb1 72d196f398
commit 1c72457215
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 136 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
plugins/woocommerce/changelog/fix-32754-inactive-downloads Normal file
View File

@ -0,0 +1,5 @@
Significance: patch
Type: tweak
Comment: We're adding extra protections to a newly introduced feature; a further changelog entry is not needed.

25
plugins/woocommerce/includes/class-wc-download-handler.php
View File

@ -31,11 +31,21 @@ class WC_Download_Handler {
* Check if we need to download a file and check validity.
*/
public static function download_product() {
// phpcs:disable WordPress.Security.NonceVerification.Recommended
$product_id = absint( $_GET['download_file'] ); // phpcs:ignore WordPress.VIP.SuperGlobalInputUsage.AccessDetected, WordPress.VIP.ValidatedSanitizedInput.InputNotValidated, WordPress.Security.ValidatedSanitizedInput.InputNotValidated
$product = wc_get_product( $product_id );
$downloads = $product ? $product->get_downloads() : array();
$data_store = WC_Data_Store::load( 'customer-download' );
if ( ! $product || empty( $_GET['key'] ) || empty( $_GET['order'] ) ) { // WPCS: input var ok, CSRF ok.
$key = empty( $_GET['key'] ) ? '' : sanitize_text_field( wp_unslash( $_GET['key'] ) );
if (
! $product
|| empty( $key )
|| empty( $_GET['order'] )
|| ! isset( $downloads[ $key ] )
|| ! $downloads[ $key ]->get_enabled()
) {
self::download_error( __( 'Invalid download link.', 'woocommerce' ) );
}
@ -43,6 +53,7 @@ class WC_Download_Handler {
if ( empty( $_GET['email'] ) && empty( $_GET['uid'] ) ) { // WPCS: input var ok, CSRF ok.
self::download_error( __( 'Invalid download link.', 'woocommerce' ) );
}
// phpcs:enable WordPress.Security.NonceVerification.Recommended
$order_id = wc_get_order_id_by_order_key( wc_clean( wp_unslash( $_GET['order'] ) ) ); // WPCS: input var ok, CSRF ok.
$order = wc_get_order( $order_id );
@ -654,10 +665,14 @@ class WC_Download_Handler {
* Since we will now render a message instead of serving a download, we should unwind some of the previously set
* headers.
*/
header( 'Content-Type: ' . get_option( 'html_type' ) . '; charset=' . get_option( 'blog_charset' ) );
header_remove( 'Content-Description;' );
header_remove( 'Content-Disposition' );
header_remove( 'Content-Transfer-Encoding' );
if ( headers_sent() ) {
wc_get_logger()->log( 'warning', __( 'Headers already sent when generating download error message.', 'woocommerce' ) );
} else {
header( 'Content-Type: ' . get_option( 'html_type' ) . '; charset=' . get_option( 'blog_charset' ) );
header_remove( 'Content-Description;' );
header_remove( 'Content-Disposition' );
header_remove( 'Content-Transfer-Encoding' );
}
if ( ! strstr( $message, '<a ' ) ) {
$message .= ' <a href="' . esc_url( wc_get_page_permalink( 'shop' ) ) . '" class="wc-forward">' . esc_html__( 'Go to shop', 'woocommerce' ) . '</a>';

115
plugins/woocommerce/tests/php/includes/class-wc-download-handler-tests.php
View File

@ -1,5 +1,7 @@
<?php
use Automattic\WooCommerce\Internal\ProductDownloads\ApprovedDirectories\Register as Approved_Directories;
/**
* Class WC_Download_Handler_Tests.
*/
@ -9,7 +11,7 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
* Test for local file path.
*/
public function test_parse_file_path_for_local_file() {
$local_file_path = trailingslashit( wp_upload_dir()['basedir'] ) . 'dummy_file.jpg';
$local_file_path = trailingslashit( wp_upload_dir()['basedir'] ) . 'dummy_file.jpg';
$parsed_file_path = WC_Download_Handler::parse_file_path( $local_file_path );
$this->assertFalse( $parsed_file_path['remote_file'] );
}
@ -18,7 +20,7 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
* Test for local URL without protocol.
*/
public function test_parse_file_path_for_local_url() {
$local_file_path = trailingslashit( wp_upload_dir()['baseurl'] ) . 'dummy_file.jpg';
$local_file_path = trailingslashit( wp_upload_dir()['baseurl'] ) . 'dummy_file.jpg';
$parsed_file_path = WC_Download_Handler::parse_file_path( $local_file_path );
$this->assertFalse( $parsed_file_path['remote_file'] );
}
@ -27,7 +29,7 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
* Test for local file with `file` protocol.
*/
public function test_parse_file_path_for_local_file_protocol() {
$local_file_path = 'file:/' . trailingslashit( wp_upload_dir()['basedir'] ) . 'dummy_file.jpg';
$local_file_path = 'file:/' . trailingslashit( wp_upload_dir()['basedir'] ) . 'dummy_file.jpg';
$parsed_file_path = WC_Download_Handler::parse_file_path( $local_file_path );
$this->assertFalse( $parsed_file_path['remote_file'] );
}
@ -36,7 +38,7 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
* Test for local file with https protocom.
*/
public function test_parse_file_path_for_local_file_https_protocol() {
$local_file_path = site_url( '/', 'https' ) . 'dummy_file.jpg';
$local_file_path = site_url( '/', 'https' ) . 'dummy_file.jpg';
$parsed_file_path = WC_Download_Handler::parse_file_path( $local_file_path );
$this->assertFalse( $parsed_file_path['remote_file'] );
}
@ -49,4 +51,109 @@ class WC_Download_Handler_Tests extends \WC_Unit_Test_Case {
$parsed_file_path = WC_Download_Handler::parse_file_path( $remote_file_path );
$this->assertTrue( $parsed_file_path['remote_file'] );
}
/**
* @testdox Customers may not use a direct download link to obtain a downloadable file that has been disabled.
*/
public function test_inactive_downloads_will_not_be_served() {
self::remove_download_handlers();
$downloads_served = 0;
$download_counter = function () use ( &$downloads_served ) {
$downloads_served++;
};
// Track downloads served.
add_action( 'woocommerce_download_file_force', $download_counter );
/**
* @var Approved_Directories $approved_directories
*/
$approved_directories = wc_get_container()->get( Approved_Directories::class );
$approved_directories->set_mode( Approved_Directories::MODE_ENABLED );
$approved_directories->add_approved_directory( 'https://always.trusted' );
$approved_directory_rule_id = $approved_directories->add_approved_directory( 'https://new.supplier' );
$product = WC_Helper_Product::create_downloadable_product(
array(
array(
'name' => 'Book 1',
'file' => 'https://always.trusted/123.pdf',
),
array(
'name' => 'Book 2',
'file' => 'https://new.supplier/456.pdf',
),
)
);
$customer = WC_Helper_Customer::create_customer();
$email = 'admin@example.org';
$order = WC_Helper_Order::create_order( $customer->get_id(), $product );
$order->set_status( 'completed' );
$order->save();
$product_id = $product->get_id();
$downloads = $product->get_downloads();
$download_keys = array_keys( $downloads );
// phpcs:disable WordPress.Security.NonceVerification.Recommended WordPress.Security.ValidatedSanitizedInput.InputNotValidated
$_GET = array(
'download_file' => $product_id,
'order' => $order->get_order_key(),
'email' => $email,
'uid' => hash( 'sha256', $email ),
'key' => $download_keys[0],
);
// With both the corresponding approved directory rules enabled...
WC_Download_Handler::download_product();
$this->assertEquals( 1, $downloads_served, 'Can successfully download "Book 1".' );
$_GET['key'] = $download_keys[1];
WC_Download_Handler::download_product();
$this->assertEquals( 2, $downloads_served, 'Can successfully download "Book 2".' );
// And now with one of the approved directory rules disabled...
$approved_directories->disable_by_id( $approved_directory_rule_id );
$_GET['key'] = $download_keys[1];
$wp_die_happened = false;
// We do not use expectException() here because we wish to continue testing after wp_die() has
// been triggered inside WC_Download_Handler::download_error().
try {
WC_Download_Handler::download_product();
} catch ( WPDieException $e ) {
$wp_die_happened = true;
}
$this->assertTrue( $wp_die_happened );
$this->assertEquals( 2, $downloads_served, 'Downloading "Book 2" failed after the corresponding approved directory rule was disabled.' );
$_GET['key'] = $download_keys[0];
WC_Download_Handler::download_product();
$this->assertEquals( 3, $downloads_served, 'Continued to be able to download "Book 1" (the corresponding rule never having been disabled.' );
// Cleanup.
add_action( 'woocommerce_download_file_force', $download_counter );
self::restore_download_handlers();
}
/**
* Unregister download handlers to prevent unwanted output and side-effects.
*/
private static function remove_download_handlers() {
remove_action( 'woocommerce_download_file_xsendfile', array( WC_Download_Handler::class, 'download_file_xsendfile' ) );
remove_action( 'woocommerce_download_file_redirect', array( WC_Download_Handler::class, 'download_file_redirect' ) );
remove_action( 'woocommerce_download_file_force', array( WC_Download_Handler::class, 'download_file_force' ) );
}
/**
* Restores download handlers in case needed by other tests.
*/
private static function restore_download_handlers() {
add_action( 'woocommerce_download_file_redirect', array( WC_Download_Handler::class, 'download_file_redirect' ), 10, 2 );
add_action( 'woocommerce_download_file_xsendfile', array( WC_Download_Handler::class, 'download_file_xsendfile' ), 10, 2 );
add_action( 'woocommerce_download_file_force', array( WC_Download_Handler::class, 'download_file_force' ), 10, 2 );
}
}