Forum OpenACS Development: ad_get_login_url Is there a security bug?

Enjoying the present moment I was reading/studying acs-tcl/tcl/security-procs.com and I noticed ad_proc ad_get_login_url returns a URL, which contains return_url, among other variables, containing user data, such as password, email and etc. It seems they are all exposed as link parameters in the login workflow process.

Meaning, ad_get_login_url executes when a user signs in/logs in and so... Are they actually exposed, when those pages get responded to the user navigation workflow through page redirections? Even if the navigation is under HTTPS those links parameters are returned to the client's browser, so they can be captured on the client's side.

Are my assumptions correct?

Is there a way we could make it more secure?

I added ns_logs and those variables were logged within error.log.

Best wishes,
I

^[[0;32m[27/Nov/2019:00:03:13][24384.7f570f7fe700][-conn:evex:6:4497-] ^[[0m^[[0;39mNotice: RETURN URL /users/password-update?old_password=57EC6E869&user_id=189002 *******^[[0m
^[[0;32m[27/Nov/2019:00:03:13][24384.7f570f7fe700][-conn:evex:6:4497-] ^[[0m^[[0;39mNotice: RETURN URL /register/?return_url=/users/password-update?old_password%3d57EC6E869%26user_id%3d189002 ******* { r\
eturn_url }^[[0m
^[[0;32m[27/Nov/2019:00:03:37][24384.7f570f7fe700][-conn:evex:6:4503-] ^[[0m^[[0;39mNotice: CALL api_src_doc {<h4><a href="/api-doc/proc-view?proc=::throttle+proc+check&source_p=1"><em>check</em></a>\
 (scripted, public)</h4><pre> ::throttle<a href='/xotcl/show-object?show_methods=1&show_source=0&object=::throttle'><mg src='/resources/acs-subsite/ZoomIn16.gif' alt='[i]' border='0'><a> check<\
/pre><blockquote><p>This method should be called once per request that is monitored.
  It should be called after authentication shuch we have already
  the userid if the user is authenticated
<p>
</blockquote>} 0 {} ::throttle proc check^[[0m
^[[0;32m[27/Nov/2019:00:03:37][24384.7f570f7fe700][-conn:evex:6:4503-] ^[[0m^[[0;39mNotice: CALL api_src_doc {<h4><a href="/api-doc/proc-view?proc=::throttle+proc+check&source_p=1"><em>check</em></a>\
 (scripted, public)</h4><pre> ::throttle<a href='/xotcl/show-object?show_methods=1&show_source=0&object=::throttle'><<mg src='/resources/acs-subsite/ZoomIn16.gif' alt='[i]' border='0'></a> check<\
/pre><blockquote><p>This method should be called once per request that is monitored.
  It should be called after authentication shuch we have already
  the userid if the user is authenticated
<p>
</blockquote>} 0 {} ::throttle proc check DONE^[[0m
^[[0;32m[27/N

If I see this correctly (the raw dump, you posted is not easy to read) the following happened above:
1) the user went to the /pvt/home page
2) pressed "change your password"
3) after filling out the form, the login expired, and got redirected to the login page (/register) with this return_url, such that after the login, the change password will be executed.

In case of redirects, all parameters (GET and POST) are turned into query parameters, since the URL passed as "location:" header field is always used in a GET issued by the browser. So, whenever a password is filled into a form there is the possibility that it might be passed at some time as a query parameter, ... and might show up in logs, etc. Therefore it is for sites with sensitive data required to use HTTPS, and server logs have to be kept secure.

This is not new (see e.g. [1]), the principle applies potentially to all return_url handling in all packages.

There are ways improvement possible, it is not clear upfront how deep some collateral damage might be.

And yes, if one enters a password in client (browser) which is compromised, the password can be lost ... in such a case the query parameters of a redirect are the least concerns.

-gn

[1] https://openacs.org/forums/message-view?message_id=187483