I have been reviewing and summarizing the patterns in recent security announcements, with an emphasis on XSS vulnerabilities as suggested by greggles.
Broadly speaking, these seem to come in a couple of forms:
- directly passing unsafe items as parameters to function calls, e.g., drupal_set_title()
- in hook_block(), setting the block['subject'] to an unsafe value
- passing unsafe items as parameters to theme() implementation functions
The first two types are easier to find and evaluate, especially if we can locate a call to check_plain() or filter_xss() (or similar) in the assignment to the variable in each case, and this assignment is in the same function body. Routines were written last year for these cases. These cases may become less common as the number of display functions in Drupal core that do not cleanse their output is reduced (or eliminated) in each version.
The third item tends to involve multiple functions and is, therefore, more difficult to find and evaluate. Based on a review of recent security announcements, the pieces tend to include:
- a page callback (interestingly, most were not to drupal_get_form())
- one or more unsafe items to display
- a theme() implementation function that is passed the unsafe strings
This pattern is found repeatedly in the recent security announcements. Those interested in this issue, please indicate whether you agree with the above assessment, whether you would like to see this pattern be coded for, and any suggestions on approach to the coding.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 1197052-notes.txt | 11.84 KB | solotandem |
Comments
Comment #1
gregglesThis analysis matches my hunches, but it would be great to see some of the research behind it if that is easy to share.
My sense is that while there are some functions where we can do filtering by default (drupal_set_title) there are not many more where we can. So it would still be valuable to look for those.
The blog['subject'] issue was I think a common one that grendzy or mr.baileys found via grep, so they may be less common than our recent history suggests.
I think another relatively common avenue for XSS is in elements of the Form API. It should be relatively easy to look for these, similarly to how you propose looking for the vulnerabilities in theme functions, right? They are detailed on http://drupal.org/node/28984
I can't think of a great general way to code for the theme issue you mention to work on all theme functions, but looking for specific theme functions should be possible, right?
Comment #2
solotandem commentedA fourth pattern is the lack of form tokens in AJAX response code leading to CSRF vulnerabilities. (This should be in a separate issue.) Examples of this pattern and the three patterns described in #1 follow. Details on each can be found in the attached research file.
Pattern #1: parameters to function calls
SA-CORE-2011-001 - Drupal core - Multiple vulnerabilities
- http://drupal.org/node/1168756
- drupal_set_message()
SA-CONTRIB-2011-020 - Taxonomy Access Control Lite (tac_lite) - Cross Site Scripting
- http://drupal.org/node/1154556
- _taxonomy_term_select()
SA-CONTRIB-2010-113 - Image - Cross Site Scripting
- http://drupal.org/node/1005578
- drupal_set_title()
Pattern #2: block['subject']
SA-CONTRIB-2011-014 - Webform Block - Cross Site Scripting
- http://drupal.org/node/1103122
SA-CONTRIB-2011-006 - Flag Page - Cross Site Scripting (XSS)
- http://drupal.org/node/1049042
Pattern #3: parameters to theme() implementation functions
SA-CONTRIB-2011-021 - Webform - Multiple Vulnerabilities
- http://drupal.org/node/1161954
SA-CONTRIB-2011-019 - Menu Access - Cross Site Scripting
- http://drupal.org/node/1147194
SA-CONTRIB-2011-013 - Tagadelic - Cross Site Scripting (XSS)
- http://drupal.org/node/1095030
SA-CONTRIB-2011-010 - Messaging - Cross Site Scripting
- http://drupal.org/node/1064024
SA-CONTRIB-2011-008 - Chatroom - Cross Site Scripting (XSS) and Cross Site Request Forgery
- http://drupal.org/node/1048990
SA-CONTRIB-2011-007 - Userpoints Cross Site Scripting
- http://drupal.org/node/1048944
SA-CONTRIB-2011-002 - Panels - Cross Site Scripting (XSS)
- http://drupal.org/node/1024972
Pattern #4: lack of form tokens in AJAX response code
SA-CONTRIB-2011-015 - Translation Management - Multiple Vulnerabilities
- http://drupal.org/node/1111174
SA-CONTRIB-2011-008 - Chatroom - Cross Site Scripting (XSS) and Cross Site Request Forgery
- http://drupal.org/node/1048990
Comment #3
gregglesRe-titling after discussion in irc. Our strategy will be to use this to identify kinds of vulnerabilities to search for and then create sub-issues for the actual code to find them.
Regarding CSRF: One strategy is to look for menu router items that don't seem to check for a valid token and yet do take some action (doing a db_query to insert/update, calling node_delete or some other function that takes an action). This is a little tricky because it could be possible to check for the token in the access callback or the page callback or in a sub-function.
Comment #4
pwolanin commentedDo we have a standardized way to check for a token in core? I thought it was usually being added at a GET argument.
Comment #5
gregglesIt is usually as a GET argument, but not necessarily (some modules do Ajax requests in a POST so the token will be in a POST).
There isn't a standard way to see if a token is present, afaik.
Comment #6
pwolanin commentedPerhaps we can try to standardize on $_REQUEST['token'] as the thing to check, or something similar? 'valid_token' or 'echt_token'
For D8 it would be interesting to see if there is a way to combine this with "form_token" in forms.