Configuration: a site without a somemodule
module. User logged in as admin.
The path admin/build/block/configure/somemodule/somedelta
opens a seemingly valid block configuration form, although there is no such block available to configure. Once the available parameters are submitted, it seems that nothing is actually changed in the DB.
The same behaviour is present in 5.2.
The same behaviour is present in 4.7.7 with paths admin/block/configure/somemodule/somedelta
It seems a normal behaviour for these paths would be to return a non existent block error, or maybe act as an alias for admin/build/block/add
.
Comment | File | Size | Author |
---|---|---|---|
#8 | exist_to_config_block_3.patch | 1.81 KB | Pasqualle |
#8 | exist_to_config_block_4.patch | 1.36 KB | Pasqualle |
#2 | exit_to_config_block_2.patch | 2.55 KB | pwolanin |
#1 | exist_to_config_block_1.patch | 1.28 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedhere's a patch for 6.x - I'm not totally happy with it though, since it gives 'Access denied' when 'Page not found' would be more accurate.
Comment #2
pwolanin CreditAttribution: pwolanin commentedHere's an alternate approach - better, worse, or just different?
Note - the fix to function block_admin_display() should be applied in either case.
Comment #3
Dries CreditAttribution: Dries commentedI'm not sure I understand this bug report. Are you saying that people can access invalid/non-existing blocks by manually altering the URL?
Comment #4
fgmNot generic "people", only admins: without the patch, these URLs, which should be nonexistent, return an apparently normal block configuration form which, when validated, does nothing visible.
Non-admins receive the normal access denied page, so it does not seem to be a vulnerability.
Comment #5
nedjoNot a huge issue but worth fixing.
Agreed that not found would be better than access denied. Could/should we instead introduce validation and conditionally return MENU_NOT_FOUND in the existing callback?
I don't follow what these lines do or why they're there in the second patch.
Comment #6
pwolanin CreditAttribution: pwolanin commented@nedjo: look at the function definition: http://api.drupal.org/api/function/block_admin_display/6
A $theme variable is passed in - which is user input. Thus the code you point to I added in order to validate that the theme exists before setting the site to use that theme...
The second patch will give "Page not found" - I should have said so. that's the point of defining the load function.
Comment #7
catchneeds a re-roll.
Comment #8
Pasquallethe patch in comment #2 is an api change, so I had to roll the patch in #1
test:
1. try: admin/build/block/configure (this does not exists, but empty block config form is displayed)
2. try: admin/build/block/configure/nomodule (this does not exists, but empty block config form is displayed)
3. try: admin/build/block/configure/system (this is powered by drupal block)
4. try: admin/build/block/configure/system/666 (this does not exists, but empty block config form is displayed)
5. apply patch
6. admin/settings/performance ->clear cached data
7. try: admin/build/block/configure (should be access denied, fallback to admin/build/block)
8. try: admin/build/block/configure/nomodule (should be access denied, fallback to admin/build/block)
9. try: admin/build/block/configure/system (should be powered by drupal block)
10. try: admin/build/block/configure/system/666 (should be access denied, fallback to admin/build/block/configure/system)
problems with patch#1:
the page arguments are not passed to block_admin_configure (fixed)
7, 8, 9 will result in
why?
in test 7, the access denied and the menu system fallback system will result in page admin/build/block with page argument configure, which tries to show that page with theme name configure
8, 9 same problem
tried to fix this, but the patch is not good enough
in patch_3 everything works as expected, the only problem is that the fallback on access denied pages does not work in tests 7, 8, 10, and I don't really know how to fix this
in patch_4 (original patch with page argument fix only) the fallback mostly works, but all tests 7, 8, 9, 10 failed.
7: php notice, fallback ok
8: php notice, fallback ok
9: the block is not displayed
10: access denied ok, fallback failed
I thought it will be an easy fix, but I am lost now..
Comment #9
dpearcefl CreditAttribution: dpearcefl commentedIs this still a problem in current D6?
Comment #10
dpearcefl CreditAttribution: dpearcefl commentedWe want your patch if it is still needed. Please resubmit it with a proper filename.
http://drupal.org/node/1054616
[description]-[issue-number]-[comment-number].patch