Skip to content

Commit 9a4a492

Browse files
aaronwareaaronware
committed
improve(NO-TASK): Minor security hardening for path traversal
1 parent 3fd3a9c commit 9a4a492

File tree

2 files changed

+86
-57
lines changed

2 files changed

+86
-57
lines changed

‎includes/Core/View.php‎

Lines changed: 73 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,12 @@ public function render( $file, $view_dir = null ) {
5757
${$key} = $value;
5858
}
5959

60-
$view_dir = isset( $view_dir ) ? trailingslashit( $view_dir ) : COURIER_NOTICES_PATH . 'templates/';
61-
$view_file = $view_dir . $file . '.php';
60+
$view_dir = isset( $view_dir ) ? $view_dir : COURIER_NOTICES_PATH . 'templates/';
61+
$view_file = $this->validate_and_build_path( $file, $view_dir );
62+
63+
if ( false === $view_file ) {
64+
wp_die( esc_html__( 'Invalid file path', 'courier-notices' ) . ': ' . esc_html( $file ) );
65+
}
6266

6367
if ( ! file_exists( $view_file ) ) {
6468
wp_die( esc_html( $view_file ) );
@@ -88,7 +92,7 @@ public function assign( $variable, $value ) {
8892
*
8993
* @since 1.0
9094
*
91-
* @param string $file File to get HTML string.
95+
* @param string $file File to get HTML string.
9296
* @param string $view_dir View directory.
9397
*
9498
* @return string $html HTML output as string
@@ -99,7 +103,11 @@ public function get_text_view( $file, $view_dir = null ) {
99103
}
100104

101105
$view_dir = isset( $view_dir ) ? $view_dir : COURIER_NOTICES_PATH . 'templates/';
102-
$view_file = $view_dir . $file . '.php';
106+
$view_file = $this->validate_and_build_path( $file, $view_dir );
107+
108+
if ( false === $view_file ) {
109+
return '';
110+
}
103111

104112
if ( ! file_exists( $view_file ) ) {
105113
return '';
@@ -125,4 +133,65 @@ public function get_text_view( $file, $view_dir = null ) {
125133
protected function init_assignments() {
126134
$this->variables = [];
127135
}
136+
137+
/**
138+
* Validate and build secure file path
139+
*
140+
* @since 1.8.0
141+
*
142+
* @param string $file The file name/path.
143+
* @param string $view_dir The view directory.
144+
*
145+
* @return string|false The validated file path or false if invalid.
146+
*/
147+
protected function validate_and_build_path( string $file, string $view_dir ) {
148+
// Validate input parameters.
149+
if ( ! is_string( $file ) || '' === $file ) {
150+
return false;
151+
}
152+
153+
if ( ! is_string( $view_dir ) || '' === $view_dir ) {
154+
return false;
155+
}
156+
157+
// Normalize the view directory to absolute path.
158+
$view_dir = realpath( $view_dir );
159+
if ( false === $view_dir ) {
160+
return false;
161+
}
162+
163+
// Ensure view directory is within the plugin directory.
164+
$plugin_path = realpath( COURIER_NOTICES_PATH );
165+
if ( false === $plugin_path || 0 !== strpos( $view_dir, $plugin_path ) ) {
166+
return false;
167+
}
168+
169+
// Remove any directory traversal attempts.
170+
$file = str_replace( [ '../', '..\\', './', '.\\' ], '', $file );
171+
172+
// Remove any null bytes or other dangerous characters.
173+
$file = str_replace( [ "\0", "\r", "\n" ], '', $file );
174+
175+
// Ensure file doesn't start with a slash or backslash.
176+
$file = ltrim( $file, '/\\' );
177+
178+
// Validate file name - allow alphanumeric, hyphens, underscores, forward slashes, and dots
179+
// but prevent directory traversal and other dangerous patterns.
180+
if ( ! preg_match( '/^[a-zA-Z0-9\/_-]+$/', $file ) ) {
181+
return false;
182+
}
183+
184+
// Build the full file path with .php extension, ensuring proper path separators.
185+
$view_file = rtrim( $view_dir, '/\\' ) . DIRECTORY_SEPARATOR . $file . '.php';
186+
187+
// Normalize the final path.
188+
$real_file_path = realpath( $view_file );
189+
190+
// Ensure the final path is within the allowed directory.
191+
if ( false === $real_file_path || 0 !== strpos( $real_file_path, $view_dir ) ) {
192+
return false;
193+
}
194+
195+
return $real_file_path;
196+
}
128197
}

‎phpstan-baseline.neon‎

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,61 +1075,11 @@ parameters:
10751075
count: 1
10761076
path: includes/Controller/Integrations/Stream.php
10771077

1078-
-
1079-
message: "#^Call to static method add_command\\(\\) on an unknown class WP_CLI\\.$#"
1080-
count: 1
1081-
path: includes/Controller/Integrations/WP_CLI.php
1082-
1083-
-
1084-
message: "#^Call to static method error\\(\\) on an unknown class WP_CLI\\.$#"
1085-
count: 10
1086-
path: includes/Controller/Integrations/WP_CLI.php
1087-
1088-
-
1089-
message: "#^Call to static method log\\(\\) on an unknown class WP_CLI\\.$#"
1090-
count: 5
1091-
path: includes/Controller/Integrations/WP_CLI.php
1092-
1093-
-
1094-
message: "#^Call to static method success\\(\\) on an unknown class WP_CLI\\.$#"
1095-
count: 2
1096-
path: includes/Controller/Integrations/WP_CLI.php
1097-
1098-
-
1099-
message: "#^Call to static method warning\\(\\) on an unknown class WP_CLI\\.$#"
1100-
count: 2
1101-
path: includes/Controller/Integrations/WP_CLI.php
1102-
11031078
-
11041079
message: "#^Cannot access offset 0 on non\\-empty\\-array\\<WP_Term\\>\\|WP_Error\\.$#"
11051080
count: 6
11061081
path: includes/Controller/Integrations/WP_CLI.php
11071082

1108-
-
1109-
message: "#^Cannot access property \\$post_content on WP_Post\\|null\\.$#"
1110-
count: 1
1111-
path: includes/Controller/Integrations/WP_CLI.php
1112-
1113-
-
1114-
message: "#^Cannot access property \\$post_date on WP_Post\\|null\\.$#"
1115-
count: 1
1116-
path: includes/Controller/Integrations/WP_CLI.php
1117-
1118-
-
1119-
message: "#^Cannot access property \\$post_modified on WP_Post\\|null\\.$#"
1120-
count: 1
1121-
path: includes/Controller/Integrations/WP_CLI.php
1122-
1123-
-
1124-
message: "#^Cannot access property \\$post_status on WP_Post\\|null\\.$#"
1125-
count: 2
1126-
path: includes/Controller/Integrations/WP_CLI.php
1127-
1128-
-
1129-
message: "#^Cannot access property \\$post_title on WP_Post\\|null\\.$#"
1130-
count: 2
1131-
path: includes/Controller/Integrations/WP_CLI.php
1132-
11331083
-
11341084
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
11351085
count: 4
@@ -1236,17 +1186,17 @@ parameters:
12361186
path: includes/Controller/Integrations/WP_CLI.php
12371187

12381188
-
1239-
message: "#^Parameter \\#1 \\$post of function get_the_terms expects int\\|WP_Post, bool\\|int given\\.$#"
1189+
message: "#^Parameter \\#1 \\$post of function get_the_terms expects int\\|WP_Post, int\\<min, \\-1\\>\\|int\\<1, max\\>\\|true given\\.$#"
12401190
count: 4
12411191
path: includes/Controller/Integrations/WP_CLI.php
12421192

12431193
-
1244-
message: "#^Parameter \\#1 \\$post_id of function update_post_meta expects int, bool\\|int given\\.$#"
1194+
message: "#^Parameter \\#1 \\$post_id of function update_post_meta expects int, int\\<min, \\-1\\>\\|int\\<1, max\\>\\|true given\\.$#"
12451195
count: 1
12461196
path: includes/Controller/Integrations/WP_CLI.php
12471197

12481198
-
1249-
message: "#^Parameter \\#1 \\$post_id of method CourierNotices\\\\Controller\\\\Action_Scheduler\\:\\:schedule_notice_expiration\\(\\) expects int, bool\\|int given\\.$#"
1199+
message: "#^Parameter \\#1 \\$post_id of method CourierNotices\\\\Controller\\\\Action_Scheduler\\:\\:schedule_notice_expiration\\(\\) expects int, int\\<min, \\-1\\>\\|int\\<1, max\\>\\|true given\\.$#"
12501200
count: 1
12511201
path: includes/Controller/Integrations/WP_CLI.php
12521202

@@ -1575,6 +1525,11 @@ parameters:
15751525
count: 1
15761526
path: includes/Core/Database.php
15771527

1528+
-
1529+
message: "#^Call to function is_string\\(\\) with string will always evaluate to true\\.$#"
1530+
count: 2
1531+
path: includes/Core/View.php
1532+
15781533
-
15791534
message: "#^Constructor of class CourierNotices\\\\Core\\\\View has an unused parameter \\$config\\.$#"
15801535
count: 1
@@ -1590,6 +1545,11 @@ parameters:
15901545
count: 1
15911546
path: includes/Core/View.php
15921547

1548+
-
1549+
message: "#^Only booleans are allowed in a negated boolean, int\\|false given\\.$#"
1550+
count: 1
1551+
path: includes/Core/View.php
1552+
15931553
-
15941554
message: "#^Property CourierNotices\\\\Core\\\\View\\:\\:\\$variables type has no value type specified in iterable type array\\.$#"
15951555
count: 1

0 commit comments

Comments
 (0)