Noticed because of #327938-70: Rules Integration, our calls to http://api.drupal.org/api/function/check_markup are wrong. By default, they check input format access for the *currently logged in user*, which is nonsense since you are supposed to be able to view a filtered content even if you don't have access to that filter. See for example node_prepare() which does it correctly: http://api.drupal.org/api/function/node_prepare/6
Proposed change as discussed in #irc:
1. set $check to FALSE for both check_markup() calls (message display and mail notification)
2. Maybe: Add a check to privatemsg_validate_message() which checks if the author does have access to the chosen filter.
3. use http://api.drupal.org/api/function/filter_resolve_format/6 to set the default format
FYI: Why this is not a security issue:
Access is checked when sending a message through the website, only the api functions don't contain an access check. That is ok since you can do anything if you can call PHP functions. However, since we do all sorts of access and validation checks in new_thread/reply I think we should also add a filter access check (there is also _privatemsg_new() if you need to save a message without api checks)
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | privatemsg_fix_check_filter_with_test.patch | 7.83 KB | berdir |
Comments
Comment #1
berdirOk, attached is a patch.
Changed all three points from above and also added tests for the validation message and, more important, to verify that the markup check does actually work.
The new tests fail without the change.
Comment #2
naheemsays commentedread through the patch and tested it via the GUI - it makes sense and works too. I did not test the API function though it also seems to be logical.
Having a look at the api, filter_access() has an $ account variable for Drupal 7, so the patch will need to be less hacky when ported to Drupal 7.
Comment #3
berdirYeah, Drupal 7 rocks again, not only did the bug not even exist there, it also a lot easier to do the validation, as you've pointed out. Fixed in both dev versions.