On Sat, 11 Jul 2015 at 18:57:58, Marcel Korpel wrote:
This fixes a bug where the new user name input by the user was invalid, causing the account deletion link and the form action to be wrong.
Signed-off-by: Marcel Korpel <marcel.korpel@gmail.com> --- web/html/account.php | 2 +- web/lib/acctfuncs.inc.php | 29 ++++++++++++++++++++++++++--- web/template/account_edit_form.php | 4 ++-- 3 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/web/html/account.php b/web/html/account.php index c447de3..8545478 100644 --- a/web/html/account.php +++ b/web/html/account.php @@ -100,7 +100,7 @@ if (isset($_COOKIE["AURSID"])) { in_request("E"), in_request("P"), in_request("C"), in_request("R"), in_request("L"), in_request("I"), in_request("K"), in_request("PK"), in_request("J"), - in_request("ID")); + in_request("ID"), $row["Username"]); } } else { if (has_credential(CRED_ACCOUNT_SEARCH)) { diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index edd38ee..ad907df 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -39,6 +39,25 @@ function html_format_pgp_fingerprint($fingerprint) { }
/** + * If a user name from the database is provided, use that one, as the other + * one might be tampered by the end user + * + * @param string $U The username, possibly provided by the user + * @param string $N The username as present in the database + * + * @return string A username that's always safe to use in URLs + */ +function create_safe_url_username($U, $N) { + // if a display username is provided but not one extracted + // from the database, use the former + if (!empty($U) && empty($N)) { + $N = $U; + } + + return $N; +} + +/** * Loads the account editing form, with any values that are already saved * * @global array $SUPPORTED_LANGS Languages that are supported by the AUR @@ -56,13 +75,15 @@ function html_format_pgp_fingerprint($fingerprint) { * @param string $PK The list of SSH public keys * @param string $J The inactivity status of the displayed user * @param string $UID The user ID of the displayed user + * @param string $N The username as present in the database * * @return void */ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", - $L="",$I="",$K="",$PK="",$J="", $UID=0) { + $L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS;
+ $N = create_safe_url_username($U, $N);
Could you explain why this is needed? Doesn't $N always refer to the correct user name? Related comment below.
include("account_edit_form.php"); return; } @@ -86,13 +107,15 @@ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="", * @param string $PK The list of public SSH keys * @param string $J The inactivity status of the user * @param string $UID The user ID of the modified account + * @param string $N The username as present in the database * * @return string|void Return void if successful, otherwise return error */ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", - $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0) { + $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") { global $SUPPORTED_LANGS;
+ $N = create_safe_url_username($U, $N); $error = '';
if (is_ipbanned()) { @@ -247,7 +270,7 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="", if ($error) { print "<ul class='errorlist'><li>".$error."</li></ul>\n"; display_account_form($A, $U, $T, $S, $E, "", "", - $R, $L, $I, $K, $PK, $J, $UID); + $R, $L, $I, $K, $PK, $J, $UID, $N);
I am confused. We only call display_account_form() here if an error occurs *before* updating the account. Shouldn't we *always* use $N here, i.e. drop the create_safe_url_username() invocation above?
return; }
diff --git a/web/template/account_edit_form.php b/web/template/account_edit_form.php index 56bdd45..0aadb9d 100644 --- a/web/template/account_edit_form.php +++ b/web/template/account_edit_form.php @@ -1,9 +1,9 @@ <?php if ($A == "UpdateAccount"): ?> <p> - <?= __('Click %shere%s if you want to permanently delete this account.', '<a href="' . get_user_uri($U) . 'delete/' . '">', '</a>') ?> + <?= __('Click %shere%s if you want to permanently delete this account.', '<a href="' . get_user_uri($N) . 'delete/' . '">', '</a>') ?> </p>
-<form id="edit-profile-form" action="<?= get_user_uri($U) . 'update/'; ?>" method="post"> +<form id="edit-profile-form" action="<?= get_user_uri($N) . 'update/'; ?>" method="post"> <?php else: ?> <form id="edit-profile-form" action="<?= get_uri('/register/'); ?>" method="post"> <?php endif; ?> -- 2.4.5