From: David H. <hic...@op...> - 2010-10-22 11:33:31
|
On Wed, 2010-10-20 at 18:42 +0200, dam...@me... wrote: > Please find attached a patch for #11351, against the master-1.2.x. It > contains a fix for both the account_page.php and the > manage_user_edit_page.php issues mentioned in the bug report. Thanks for the patch Damien. I have applied a modified version of your patch to both 1.2.x and 1.3.x branches. Your patch essentially relied upon the user submitting their current LDAP email/real name to the account update scripts. The user cannot be trusted in such situations and therefore I have redone the patch. Now we don't send any hidden fields at all if LDAP is enabled for emails/real names. Instead, the scripts receiving the forms will either not update LDAP-enforced fields or will pull values from LDAP moments before inserting rows into the database. > I have successfully tested all cases (with DB auth, and with LDAP auth > and all 4 combinations of the two parameters use_ldap_email / > use_ldap_realname. Can you please retest the latest 1.2.x and/or 1.3.x branches to see if everything still works OK with my changes? > Note, I'm quite new to git and there was a merge conflict with > account_page.php, so I hope I handled it properly. There was a problem with a missing PHP opening/closing tag so in the future I suggest switching on maximum PHP error and warning verbosity and testing files before rolling up a patch. Nothing I couldn't fix in a few keystrokes though! I did come across a few unescaped variables (user names, real names, variables) that were being outputted straight to HTML. These have the potential to be XSS (cross site scripting) vulnerabilities and therefore we should always be using string_attribute() to escape variables when setting default HTML form input "value" attributes. When printing user names, etc to the screen as plain text, we need to use string_display_line() or string_display() depending on whether it is one line of text you're expecting or multiple lines. > Hopefully one of you guys can review my work and mark this issue as > resolved. I have marked issue #11351 as resolved for now. If there are any further problems with the changes I've made, please reopen. Thanks again for your contribution to MantisBT. Regards, David |