Skip to content
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.

Sign up for GitHub

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

Limit directories that can be deleted #1557

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from
Draft

Limit directories that can be deleted #1557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion src/Helper/Helper_Misc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,61 @@
* @return bool|WP_Error
*
* @since 4.0
* @since 6.12.0 Directories not managed by Gravity PDF will throw an error if tried to be deleted
*/
public function rmdir( $dir, bool $delete_top_level_dir = true ) {

/*
* Do not allow directories outside the folders managed by Gravity PDF to be deleted
*/
$folders = [
$this->data->template_location,
$this->data->template_font_location,
$this->data->template_tmp_location,
$this->data->mpdf_tmp_location,
];

if ( is_multisite() ) {
$folders[] = $this->data->multisite_template_location;

Check warning on line 371 in src/Helper/Helper_Misc.php

View check run for this annotation

Codecov / codecov/patch

src/Helper/Helper_Misc.php#L371 

Added line #L371 was not covered by tests
}

/* Verify $dir is a real path on the current file system */
$path_to_test = realpath( $dir );
if ( $path_to_test === false ) {
$this->log->error(
'Filesystem Delete Error',
[
'dir' => $dir,
'exception' => 'Not a real path on the file system',
]
);

return new WP_Error( 'rmdir_not_a_real_path', '' );
}

/* Check if $dir to delete falls inside one of the Gravity PDF directories */
$allowed_to_delete = false;
foreach ( $folders as $folder ) {

if ( strpos( $path_to_test, realpath( $folder ) ) === 0 ) {
$allowed_to_delete = true;
break;
}
}

if ( ! $allowed_to_delete ) {
$this->log->error(
'Filesystem Delete Error',
[
'dir' => $dir,
'exception' => 'Directory falls outside of approved paths',
]
);

return new WP_Error( 'rmdir_directory_not_approved', esc_html( 'Cannot delete path. Directory falls outside of approved paths: ' . $dir ) );
}

/* Path is managed by Gravity PDF and can be deleted */
$this->log->notice( sprintf( 'Begin deleting directory recursively: %s', $dir ) );

try {
Expand All @@ -368,7 +420,7 @@
$function = ( $fileinfo->isDir() ) ? 'rmdir' : 'unlink';
$real_path = $fileinfo->getRealPath();
if ( ! $function( $real_path ) ) {
throw new Exception( 'Could not run ' . $function . ' on ' . $real_path );
throw new Exception( esc_html( 'Could not run ' . $function . ' on ' . $real_path ) );

Check warning on line 423 in src/Helper/Helper_Misc.php

View check run for this annotation

Codecov / codecov/patch

src/Helper/Helper_Misc.php#L423 

Added line #L423 was not covered by tests
}

$this->log->notice( sprintf( 'Successfully ran `%s` on %s', $function, $real_path ) );
Expand Down
64 changes: 32 additions & 32 deletions src/Model/Model_Install.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,25 +182,27 @@
$upload_dir_url = $this->data->upload_dir_url;

/* Legacy Filters */
$this->data->template_location = apply_filters( 'gfpdfe_template_location', $template_dir, $working_folder, $upload_dir );
$this->data->template_location_url = apply_filters( 'gfpdfe_template_location_uri', $template_url, $working_folder, $upload_dir_url );
$this->data->template_location = trailingslashit( apply_filters( 'gfpdfe_template_location', $template_dir, $working_folder, $upload_dir ) );
$this->data->template_location_url = trailingslashit( apply_filters( 'gfpdfe_template_location_uri', $template_url, $working_folder, $upload_dir_url ) );

/* Allow user to change directory location(s) */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_template_location/ for more details about this filter */
$this->data->template_location = apply_filters( 'gfpdf_template_location', $this->data->template_location, $working_folder, $upload_dir ); /* needs to be accessible from the web */
$this->data->template_location = trailingslashit( apply_filters( 'gfpdf_template_location', $this->data->template_location, $working_folder, $upload_dir ) ); /* needs to be accessible from the web */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_template_location_uri/ for more details about this filter */
$this->data->template_location_url = apply_filters( 'gfpdf_template_location_uri', $this->data->template_location_url, $working_folder, $upload_dir_url ); /* needs to be accessible from the web */
$this->data->template_location_url = trailingslashit( apply_filters( 'gfpdf_template_location_uri', $this->data->template_location_url, $working_folder, $upload_dir_url ) ); /* needs to be accessible from the web */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_font_location/ for more details about this filter */
$this->data->template_font_location = apply_filters( 'gfpdf_font_location', $this->data->template_location . 'fonts/', $working_folder, $upload_dir ); /* can be in a directory not accessible via the web */
$this->data->template_font_location = trailingslashit( apply_filters( 'gfpdf_font_location', $this->data->template_location . 'fonts/', $working_folder, $upload_dir ) ); /* can be in a directory not accessible via the web */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_tmp_location/ for more details about this filter */
$this->data->template_tmp_location = apply_filters( 'gfpdf_tmp_location', $this->data->template_location . 'tmp/', $working_folder, $upload_dir_url ); /* encouraged to move this to a directory not accessible via the web */
$this->data->template_tmp_location = trailingslashit( apply_filters( 'gfpdf_tmp_location', $this->data->template_location . 'tmp/', $working_folder, $upload_dir_url ) ); /* encouraged to move this to a directory not accessible via the web */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_mpdf_tmp_location/ for more details about this filter */
$mpdf_tmp_path = $this->data->template_tmp_location . 'mpdf';
$mpdf_tmp_path = $this->data->template_tmp_location . 'mpdf';

/* mPDF requires that the path excludes the trailing slash */
$this->data->mpdf_tmp_location = untrailingslashit( apply_filters( 'gfpdf_mpdf_tmp_location', $mpdf_tmp_path ) );
}

Expand All @@ -209,38 +211,38 @@
*
* @return void
* @since 4.0
*
*/
public function setup_multisite_template_location() {

if ( is_multisite() ) {
if ( ! is_multisite() ) {
return;
}

$blog_id = get_current_blog_id();
$blog_id = get_current_blog_id();

Check warning on line 221 in src/Model/Model_Install.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Model_Install.php#L221 

Added line #L221 was not covered by tests

$template_dir = $this->data->template_location . $blog_id . '/';
$template_url = $this->data->template_location_url . $blog_id . '/';
$working_folder = $this->data->working_folder;
$upload_dir = $this->data->upload_dir;
$upload_dir_url = $this->data->upload_dir_url;
$template_dir = $this->data->template_location . $blog_id . '/';
$template_url = $this->data->template_location_url . $blog_id . '/';
$working_folder = $this->data->working_folder;
$upload_dir = $this->data->upload_dir;
$upload_dir_url = $this->data->upload_dir_url;

Check warning on line 227 in src/Model/Model_Install.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Model_Install.php#L223-L227 

Added lines #L223 - L227 were not covered by tests

/**
* Allow user to change directory location(s)
*
* @internal Folder location needs to be accessible from the web
*/
/**
* Allow user to change directory location(s)
*
* @internal Folder location needs to be accessible from the web
*/

/* Global filter */
/* Global filter */

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_multisite_template_location/ for more details about this filter */
$this->data->multisite_template_location = apply_filters( 'gfpdf_multisite_template_location', $template_dir, $working_folder, $upload_dir, $blog_id );
/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_multisite_template_location/ for more details about this filter */
$this->data->multisite_template_location = trailingslashit( apply_filters( 'gfpdf_multisite_template_location', $template_dir, $working_folder, $upload_dir, $blog_id ) );

Check warning on line 238 in src/Model/Model_Install.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Model_Install.php#L238 

Added line #L238 was not covered by tests

/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_multisite_template_location_uri/ for more details about this filter */
$this->data->multisite_template_location_url = apply_filters( 'gfpdf_multisite_template_location_uri', $template_url, $working_folder, $upload_dir_url, $blog_id );
/* See https://docs.gravitypdf.com/v6/developers/filters/gfpdf_multisite_template_location_uri/ for more details about this filter */
$this->data->multisite_template_location_url = trailingslashit( apply_filters( 'gfpdf_multisite_template_location_uri', $template_url, $working_folder, $upload_dir_url, $blog_id ) );

Check warning on line 241 in src/Model/Model_Install.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Model_Install.php#L241 

Added line #L241 was not covered by tests

/* Per-blog filters */
$this->data->multisite_template_location = apply_filters( 'gfpdf_multisite_template_location_' . $blog_id, $this->data->multisite_template_location, $working_folder, $upload_dir, $blog_id );
$this->data->multisite_template_location_url = apply_filters( 'gfpdf_multisite_template_location_uri_' . $blog_id, $this->data->multisite_template_location_url, $working_folder, $upload_dir_url, $blog_id );
}
/* Per-blog filters */
$this->data->multisite_template_location = trailingslashit( apply_filters( 'gfpdf_multisite_template_location_' . $blog_id, $this->data->multisite_template_location, $working_folder, $upload_dir, $blog_id ) );
$this->data->multisite_template_location_url = trailingslashit( apply_filters( 'gfpdf_multisite_template_location_uri_' . $blog_id, $this->data->multisite_template_location_url, $working_folder, $upload_dir_url, $blog_id ) );

Check warning on line 245 in src/Model/Model_Install.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Model_Install.php#L244-L245 

Added lines #L244 - L245 were not covered by tests
}

/**
Expand All @@ -255,9 +257,7 @@
public function create_folder_structures() {

/* don't create the folder structure on our welcome page or through AJAX as an errors on the first page they see will confuse users */
if ( is_admin() &&
( rgget( 'page' ) === 'gfpdf-getting-started' || ( defined( 'DOING_AJAX' ) && DOING_AJAX ) )
) {
if ( is_admin() && ( defined( 'DOING_AJAX' ) && DOING_AJAX ) ) {
return null;
}

Expand Down
35 changes: 31 additions & 4 deletions tests/phpunit/unit-tests/test-helper-misc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,14 +615,16 @@ public function provider_in_array() {
}

/**
* Test that the everything inside a directory gets removed
* Test that everything inside a directory gets removed
*
* @since 4.0
* @since 6.13 The cleanup routines cannot delete folders/files outside the paths managed by Gravity PDF
*/
public function test_cleanup_dir() {
$data = \GPDFAPI::get_data_class();

/* Create our test data */
$path = '/tmp/test/';
$path = $data->template_location .'folder/';
wp_mkdir_p( $path );
touch( $path . 'test' );

Expand All @@ -637,11 +639,25 @@ public function test_cleanup_dir() {
$this->assertDirectoryExists( $path );

rmdir( $path );

/* Verify we cannot delete folders outside those the plugin manages */
$path = sys_get_temp_dir() . '/folder/';
wp_mkdir_p( $path );
touch( $path . 'test' );
$this->assertFileExists( $path . 'test' );

$this->misc->cleanup_dir( $path );

$this->assertFileExists( $path . 'test' );
unlink( $path . 'test' );
rmdir( $path );
}

public function test_rmdir() {
/* Create test data */
$path = '/tmp/test/';
$data = \GPDFAPI::get_data_class();

/* Create our test data */
$path = $data->template_location .'folder/';
wp_mkdir_p( $path );
touch( $path . 'test' );

Expand All @@ -665,5 +681,16 @@ public function test_rmdir() {

$this->assertFileDoesNotExist( $path . 'test' );
$this->assertDirectoryDoesNotExist( $path );

/* Verify we cannot delete folders outside those the plugin manages */
$path = sys_get_temp_dir() . '/folder/';
wp_mkdir_p( $path );
$this->assertDirectoryExists( $path );

$results = $this->misc->rmdir( $path );
$this->assertSame( 'rmdir_directory_not_approved', $results->get_error_code() );

$this->assertDirectoryExists( $path );
rmdir( $path );
}
}