Skip to content

Commit 24737a6

Browse files
johnbillionjohnbillion
committed
Reject success URLs that contain a username credential. These are almost exclusively used for phishing and is deprecated in modern browsers.
1 parent 9d04cdc commit 24737a6

4 files changed

Lines changed: 142 additions & 4 deletions

File tree

‎src/wp-admin/authorize-application.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
}
5252

5353
if ( $redirect ) {
54-
// Explicitly not using wp_safe_redirect b/c sends to arbitrary domain.
54+
// Explicitly not using wp_safe_redirect b/c sends to arbitrary domain or custom scheme URL.
5555
wp_redirect( $redirect );
5656
exit;
5757
}

‎src/wp-admin/includes/user.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,13 @@ function wp_is_authorize_application_redirect_url_valid( $url ) {
735735
);
736736
}
737737

738+
if ( null !== wp_parse_url( $url, PHP_URL_USER ) ) {
739+
return new WP_Error(
740+
'invalid_redirect_url_format',
741+
__( 'Credentials are not allowed in the URL.' )
742+
);
743+
}
744+
738745
if ( 'http' === $scheme && ! $is_local ) {
739746
return new WP_Error(
740747
'invalid_redirect_scheme',

‎tests/phpunit/tests/admin/Admin_Includes_User_WpIsAuthorizeApplicationPasswordRequestValid_Test.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ public function data_is_authorize_application_password_request_valid() {
6464
'env' => $environment_type,
6565
);
6666

67-
$datasets[ $environment_type . ' and a "http" scheme "reject_url"' ] = array(
68-
'request' => array( 'reject_url' => 'http://example.org' ),
69-
'expected_error_code' => 'local' === $environment_type ? '' : 'invalid_redirect_scheme',
67+
$datasets[ $environment_type . ' and a userinfo "success_url"' ] = array(
68+
'request' => array( 'success_url' => 'https://user:[email protected]/capture' ),
69+
'expected_error_code' => 'invalid_redirect_url_format',
70+
'env' => $environment_type,
71+
);
72+
73+
$datasets[ $environment_type . ' and a userinfo-only "success_url"' ] = array(
74+
'request' => array( 'success_url' => 'https://[email protected]/capture' ),
75+
'expected_error_code' => 'invalid_redirect_url_format',
7076
'env' => $environment_type,
7177
);
7278
}

‎tests/phpunit/tests/admin/Admin_Includes_User_WpIsAuthorizeApplicationRedirectUrlValid_Test.php‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
<?php
2+
3+
/**
4+
* @group admin
5+
* @group user
6+
*
7+
* @covers ::wp_is_authorize_application_redirect_url_valid
8+
*/
9+
class Admin_Includes_User_WpIsAuthorizeApplicationRedirectUrlValid_Test extends WP_UnitTestCase {
10+
11+
/**
12+
* Test redirect URL validation for application password authorization.
13+
*
14+
* @ticket nnnnn
15+
*
16+
* @dataProvider data_is_authorize_application_redirect_url_valid
17+
*
18+
* @param string $url The URL to validate.
19+
* @param string $expected_error_code The expected error code, empty if no error is expected.
20+
* @param string $expected_message The expected error message, empty if no error is expected.
21+
* @param string $env The environment type. Defaults to 'production'.
22+
*/
23+
public function test_is_authorize_application_redirect_url_valid( $url, $expected_error_code, $expected_message = '', $env = 'production' ) {
24+
putenv( "WP_ENVIRONMENT_TYPE=$env" );
25+
26+
$actual = wp_is_authorize_application_redirect_url_valid( $url );
27+
28+
putenv( 'WP_ENVIRONMENT_TYPE' );
29+
30+
if ( $expected_error_code ) {
31+
$this->assertWPError( $actual, 'A WP_Error object is expected.' );
32+
$this->assertSame( $expected_error_code, $actual->get_error_code(), 'Unexpected error code.' );
33+
if ( $expected_message ) {
34+
$this->assertSame( $expected_message, $actual->get_error_message(), 'Unexpected error message.' );
35+
}
36+
} else {
37+
$this->assertNotWPError( $actual, 'A WP_Error object is not expected.' );
38+
}
39+
}
40+
41+
public function data_is_authorize_application_redirect_url_valid() {
42+
return array(
43+
// Empty URL is valid (no redirect).
44+
'empty URL' => array(
45+
'url' => '',
46+
'expected_error_code' => '',
47+
),
48+
49+
// Valid HTTPS URLs.
50+
'https URL' => array(
51+
'url' => 'https://example.org',
52+
'expected_error_code' => '',
53+
),
54+
'https URL with path' => array(
55+
'url' => 'https://example.org/callback',
56+
'expected_error_code' => '',
57+
),
58+
'https URL with port' => array(
59+
'url' => 'https://example.org:8443/callback',
60+
'expected_error_code' => '',
61+
),
62+
'https URL with query params' => array(
63+
'url' => 'https://example.org/callback?existing=param',
64+
'expected_error_code' => '',
65+
),
66+
67+
// Valid app scheme URLs.
68+
'app scheme URL' => array(
69+
'url' => 'wordpress://example',
70+
'expected_error_code' => '',
71+
),
72+
'custom app scheme URL' => array(
73+
'url' => 'myapp://callback',
74+
'expected_error_code' => '',
75+
),
76+
77+
// Userinfo in URL (authority confusion attack).
78+
'username and password in URL' => array(
79+
'url' => 'https://user:[email protected]/capture',
80+
'expected_error_code' => 'invalid_redirect_url_format',
81+
'expected_message' => 'Credentials are not allowed in the URL.',
82+
),
83+
'username only in URL' => array(
84+
'url' => 'https://[email protected]/capture',
85+
'expected_error_code' => 'invalid_redirect_url_format',
86+
'expected_message' => 'Credentials are not allowed in the URL.',
87+
),
88+
'username with empty password in URL' => array(
89+
'url' => 'https://user:@evil.com/capture',
90+
'expected_error_code' => 'invalid_redirect_url_format',
91+
'expected_message' => 'Credentials are not allowed in the URL.',
92+
),
93+
94+
// Invalid protocols.
95+
'javascript protocol' => array(
96+
'url' => 'javascript:alert(1)',
97+
'expected_error_code' => 'invalid_redirect_url_format',
98+
),
99+
'data protocol' => array(
100+
'url' => 'data:text/html,<script>alert(1)</script>',
101+
'expected_error_code' => 'invalid_redirect_url_format',
102+
),
103+
104+
// Invalid format.
105+
'no scheme' => array(
106+
'url' => 'example.org/callback',
107+
'expected_error_code' => 'invalid_redirect_url_format',
108+
),
109+
110+
// HTTP scheme depends on environment.
111+
'http URL on production' => array(
112+
'url' => 'http://example.org',
113+
'expected_error_code' => 'invalid_redirect_scheme',
114+
'expected_message' => '',
115+
'env' => 'production',
116+
),
117+
'http URL on local' => array(
118+
'url' => 'http://example.org',
119+
'expected_error_code' => '',
120+
'expected_message' => '',
121+
'env' => 'local',
122+
),
123+
);
124+
}
125+
}

0 commit comments

Comments
 (0)