Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Currently, all the admin routes for amazon_pa require only the 'access administration pages' permission. This is generally a very "low risk" admin perm, that you frequently have to give out to any role that needs to do anything admin-esque. However, since this module's settings forms only require that, you can do a lot of fairly disruptive/malicious things to a site running amazon_pa:
- Blow through the daily API quota and DoS this feature on the site.
- Silently reconfigure the site to send the referral $$ to your API key instead of the site owner's
- ...
The previous amazon module (that this was based on) had a dedicated permission for this ("administer amazon"), so that you could really lock down the settings forms to sufficiently high-powered admin roles, not just anyone that needs to do anything admin-y.
Note: I was tempted to open this bug report on security.drupal.org, but I spoke to @greggles privately and the security team agreed this can be a public issue and doesn't require the formal security announcement workflow...
Steps to reproduce
- Create a low-tier role for trivial admin tasks.
- Give it 'access administration pages' permissions.
- Login as a user with this role.
- Visit /admin/config/services/amazon
- Enjoy viewing the Amazon PA API access keys, change things you probably shouldn't be able to change, etc.
Proposed resolution
Add a dedicated permission for this.
Remaining tasks
- Do it.
- Decide if there should be an upgrade path or just document the change in the release notes.
- Reviews/refinements.
- RTBC.
- Commit.
User interface changes
Not really. Just a new permission specifically for this module.
API changes
Nope.
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3219462-2.patch | 1.52 KB | dww |
Comments
Comment #2
dwwThis applies cleanly to 8.x-1.x and 2.0.x branches.
Comment #3
marcoka commentedLooks good. Thanks.
I will add this when my git push over mistage is resolved i guess :)
Comment #4
marcoka commentedVery good point as someone can change the affiliate id and that would be a disaster. Also the api keys. never thought about that because i never give admin to anyone :). Thank you.
I decided to add "restrict access: true" because of that.
Works here, i commited it to the dev.
As this is a dev, i added info to the changelog.
Comment #6
marcoka commentedComment #8
dwwThanks! I pushed a minor follow-up fix to keep the permission machine name all lowercase, which seems to be the convention, even in the case of proper nouns like "Amazon PA" or "Twitter", etc.