- Issue created by @NotifyOne
- Status changed to Needs work
over 1 year ago 1:15pm 8 June 2023 - last update
over 1 year ago Composer require failure - Status changed to Needs review
over 1 year ago 1:24pm 8 June 2023 - Issue was unassigned.
- Status changed to RTBC
over 1 year ago 1:21pm 15 June 2023 - 🇮🇳India shyam_bhatt Gujarat
I have checked the "MR !11" working fine with the field type "managed_file".
Please check the below steps:
- Install and enable the Menu Link Attributes module.
- Go to the configuration page "/admin/config/menu_link_attributes/config".
- Add the "managed_file" field as per the below code:
icon-attr: label: 'icon label' type: managed_file upload_location: 'public://module-images/menu-link-images/' file_validate_extensions: 'gif png'
The "file_validate_extensions" key was added to the patch. so we can restrict the file with different extensions.
Please check the after image.It will not allow a jpg file.
It will allow only png and gif files.
- Assigned to jcnventura
- 🇩🇪Germany Anybody Porta Westfalica
@jcnventura: What do you think about this addition as original maintainer? I'd like to have a sign-off first, before merging this. Thanks!
- Status changed to Needs work
3 months ago 8:57am 19 August 2024 - 🇵🇹Portugal jcnventura
First, to clarify that the original maintainer is @yannickoo.
Second, this change has security implications that may not be transparent to site administrators. With this, the person responsible for managing the menu can now define both where an upload is added and the file type. I don't think it would be too hard to use this to now inject a malicious JS or PHP file in the system somewhere where it is an XSS or code injection problem.
I know it is a pain to maintain, but this should really use a hard-coded allowlist of possible extensions (jpg, jpeg, gif, png, svg, etc.) and only apply those extensions that are in the whitelist.
Also note that this may not be compatible with Drupal 11: https://www.drupal.org/node/3363700 →
This coupled to the fact that the issue summary doesn't even say why this would be a nice feature to have leave me with a lot of reservations on whether this is RTBC. I'm setting this to needs work, so that we are sure to have a system that works with the new file.validator service, and also to somehow block the security issue that I believe this is introducing.
- Issue was unassigned.
- 🇩🇪Germany Anybody Porta Westfalica
Thanks @jcnventura my feelings were the same and sorry I thought you were the original maintainer :)
- First commit to issue fork.