Suggested commit message (based on bug bounty input)
Issue #2512460 by pwolanin, Gábor Hojtsy, grisendo, JvE: "Translate user edited configuration" permission needs to be marked as restricted
Problem/Motivation
The "Translate user edited configuration" permission actually allows one to translate shipped configuration strings, which is an overlap with interface text translation. This may result in string reuse between configuration and interface which leads to these configuration translations possibly returned by t(), e.g. with translating default content type labels. The return value of t() is considered safe, so the permission to translate configuration needs to be marked restricted.
reported multiply in the Drupal 8 security bug bounty program
https://tracker.bugcrowd.com/submissions/672a7ac983d1d6e554114e2f287824a...
https://tracker.bugcrowd.com/submissions/4cab8e9ba13cfb3d4eec3348bd884b3...
Proposed resolution
The config translation permission should be clarified (the incorrect description fixed) and the interface translation one should be updated to match actual behavior. The config translation permission needs to be a restricted permission.
Remaining tasks
Review. Commit.
User interface changes
Permission names are better, descriptions are accurate. Config translation permission is restricted.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#7 | 2512460-6.patch | 1.1 KB | Gábor Hojtsy |
#7 | interdiff.txt | 1.1 KB | Gábor Hojtsy |
#7 | InterfaceTranslatioVSConfig.png | 159.93 KB | Gábor Hojtsy |
#1 | 2512460-1.patch | 523 bytes | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin as a volunteer and at Acquia commentedThis patch came from JvE on bugcrowd
Comment #2
dawehnerAgreed
Comment #3
Gábor HojtsyWe discussed with @pwolanin that the name of the perm also needs fixing for clarity.
Comment #4
Gábor HojtsyComment #5
Gábor HojtsyComment #6
pwolanin CreditAttribution: pwolanin as a volunteer and at Acquia commentedComment #7
Gábor HojtsyUpdated permission names and descriptions. This is the figure I drew for @pwolanin yesterday to explain the issue :)
Comment #8
Gábor HojtsyFix terminology
Comment #9
dawehnerThat is not a bad idea!
Comment #10
alexpottCommitted 447538d and pushed to 8.0.x. Thanks!
Comment #13
Gábor HojtsySuperb, thanks!
Comment #14
xjmSee #2512466-33: Config translation needs to be validated on input for XSS (like other t string input). Let's add a CR for this.
Comment #15
xjmComment #16
dani3lr0se CreditAttribution: dani3lr0se as a volunteer commentedI'd like to have a try with writing the change record. I'm a novice user looking for ways to help and get some experience. :)
Comment #17
dani3lr0se CreditAttribution: dani3lr0se as a volunteer commentedHere is the change record for review: https://www.drupal.org/node/2522770. Hopefully it looks ok.
Comment #18
dani3lr0se CreditAttribution: dani3lr0se as a volunteer commentedComment #19
larowlanPublished the change record - great work daniel_rose
Comment #20
dani3lr0se CreditAttribution: dani3lr0se as a volunteer commentedThanks for your help @larowlan
Comment #21
larowlanComment #22
Gábor HojtsyNote that the security policy was updated at https://www.drupal.org/node/475848/revisions/view/7267195/8630716 as well.