0006044: 'Return' _GET is not checked

(0011023)
grangeway (developer)
2005-07-29 18:53

Reminder sent to: thraxisp, vboctor

thoughts on ways to fix/check this one? :)

(0011024)
grangeway (developer)
2005-07-29 19:02

It's also an RFC violation to not include the full host iirc.

(0011077)
thraxisp (manager)
2005-08-04 20:16

The same structure also exists in login_page.php, manage_config_revert.php, manage_custom_field_delete.php, manage_custom_field_edit_page.php, and manage_custom_field_update.php.

I'd suggest that we parse the value and, if there are any slashes (/), strip off anything before the last /. We can then append it to the mantis base URL.

(0011107)
ryandesign (reporter)
2005-08-07 16:36

We could develop a function to sanitize the return value...

$f_return = sanitize_return( gpc_get_string( 'return', config_get( 'default_home_page' ) ) );

It could check if the return value is under the Mantis web URL and discard it if not.

The RFC 2616 violation when redirecting is bug 0003911. The library I've attached there can help us with the above too -- my complete_url() function can take the supposedly relative URL in the "return" parameter and convert it to a full URL, and then a comparison can be done to see if the Mantis base URL can be found starting at character 0 of that complete URL.

(0011108)
ryandesign (reporter)
2005-08-07 16:42

Although your second example, where you put a second Location header in the GET parameter, is very cute. I suspect my library does nothing to help with that, nor, after a little thought, should it, really. The path should be sanitized before passing it to my function.

(0012072)
thraxisp (manager)
2006-02-02 22:57

Fixed in CVS.

core/string_api.php -> 1.75.4.2.2.1.2.1
core/print_api.php -> 1.143.6.1.4.1
core/html_api.php -> 1.184.4.1.2.1.2.1

Issue History
Date Modified	Username	Field	Change
2005-07-29 18:51	grangeway	New Issue
2005-07-29 18:53	grangeway	Note Added: 0011023
2005-07-29 19:02	grangeway	Note Added: 0011024
2005-08-04 20:16	thraxisp	Note Added: 0011077
2005-08-07 16:36	ryandesign	Note Added: 0011107
2005-08-07 16:42	ryandesign	Note Added: 0011108
2006-02-02 22:43	thraxisp	Issue cloned: 0006665
2006-02-02 22:43	thraxisp	Relationship added	parent of 0006665
2006-02-02 22:57	thraxisp	Status	new => resolved
2006-02-02 22:57	thraxisp	Fixed in Version	=> 1.0.0rc6
2006-02-02 22:57	thraxisp	Resolution	open => fixed
2006-02-02 22:57	thraxisp	Assigned To	=> thraxisp
2006-02-02 22:57	thraxisp	Note Added: 0012072
2006-02-04 05:44	vboctor	Status	resolved => closed
2006-10-09 11:55	thraxisp	View Status	private => public

Notes
(0011023) grangeway (developer) 2005-07-29 18:53	Reminder sent to: thraxisp, vboctor thoughts on ways to fix/check this one? :)

(0011024) grangeway (developer) 2005-07-29 19:02	It's also an RFC violation to not include the full host iirc.

(0011077) thraxisp (manager) 2005-08-04 20:16	The same structure also exists in login_page.php, manage_config_revert.php, manage_custom_field_delete.php, manage_custom_field_edit_page.php, and manage_custom_field_update.php. I'd suggest that we parse the value and, if there are any slashes (/), strip off anything before the last /. We can then append it to the mantis base URL.

(0011107) ryandesign (reporter) 2005-08-07 16:36	We could develop a function to sanitize the return value... $f_return = sanitize_return( gpc_get_string( 'return', config_get( 'default_home_page' ) ) ); It could check if the return value is under the Mantis web URL and discard it if not. The RFC 2616 violation when redirecting is bug 0003911. The library I've attached there can help us with the above too -- my complete_url() function can take the supposedly relative URL in the "return" parameter and convert it to a full URL, and then a comparison can be done to see if the Mantis base URL can be found starting at character 0 of that complete URL.

(0011108) ryandesign (reporter) 2005-08-07 16:42	Although your second example, where you put a second Location header in the GET parameter, is very cute. I suspect my library does nothing to help with that, nor, after a little thought, should it, really. The path should be sanitized before passing it to my function.

(0012072) thraxisp (manager) 2006-02-02 22:57	Fixed in CVS. core/string_api.php -> 1.75.4.2.2.1.2.1 core/print_api.php -> 1.143.6.1.4.1 core/html_api.php -> 1.184.4.1.2.1.2.1