Unable to upload of user profile pictures in bulk when username contains special characters

removing the validation for clean_param for filename.

Sam what's your opinion with the solution?

Rosie

Hi Rossi,
By the looks of it the clean_param is required to ensure that the files contained in the uploaded zip files don't have malicious file names.
I certainly think we need to clean/check them, perhaps it can be done in the same way that we process usernames when uploading users, was there a PARAM_ that you used for that?
I've added Petr as watcher:
Petr, perhaps you could advise us about the security issues of using a more lenient check?

Thanks for the review Sam.

The reason, i removed it because with 'User attribute to use to match pictures: username' the file name is used to match with the username in DB. if a user was created with extendedusernamechars enabled, then username may contain special characters such as #$~. if we use validation on the file name to eliminate malicious character, the system will skip uploading profile picture for the user.

I did create PARAM_USERNAME to validate username, however it relies on the current setting for extenedusernamechars. a user maybe created with the extendedchars enable and disable it right after. in this case, uploading profile picture will be skip for that user.

Note for petr: PARAM_USERNAME has not been committed yet. Please refer to MDL-16919 for reference.

The file names are cleaned during unzipping, I doubt there is any way to compensate for loss of this information later, so the current code can not detect it. The patch makes sense to me, and the problematic pictures have to end with warning notify(get_string('uploadpicture_usernotfound', 'admin', $a));

I personally think that people should not use funny characters in usernames, and if they do they should setup some meaningful user IDnumbers and use them instead of usernames when uploading pictures.

So +1 for commit

Petr, Thank you for the input.

committed patch to 19_stable and HEAD.