Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Recent patches for Form API, Filter module, and whatnot made me worry about D7's healthiness with regard to sanitation of user input on output.
The idea:
1) Enable all modules.
2) Grant all permissions.
3) Take menu router, fetch all router items having 'page callback' => 'drupal_get_form'.
4) Get all of those forms and insert <script>alert('XSS');</script> into all textfields and textareas. Submit.
5) Handle validation errors somehow. (Magic xpath trickery, but possible.)
6) Afterwards, fetch the menu router again, and visit any possible page in Drupal.
7) On every single page, assert that <script>alert('XSS');</script> is not contained in the raw output.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | drupal.security-xss.8.patch | 15.32 KB | sun |
| #7 | drupal.security-xss.7.patch | 15.13 KB | sun |
| #6 | drupal.security-xss.5.patch | 11.09 KB | sun |
| #4 | drupal.security-xss.4.patch | 7.74 KB | sun |
| #3 | drupal.security-xss.3.patch | 7.68 KB | sun |
Comments
Comment #1
sunBasically we want to move http://drupal.org/project/security_scanner into a test in core. Thanks dmitrig01!
Comment #3
sunHeavily advanced.
Comment #4
sunFixed default local task omission.
Comment #6
sunNext to some real XSS security issues, this patch nicely reveals some other logic errors and broken code in Drupal core.
I highly recommend to enable debug/verbose output in SimpleTest, run this test locally, and look at the results... the test is able to insert the XSS values in fields where they should not be accepted in the first place at all. Oh my, lots of broken code. :)
Comment #7
sunFirst round of eliminating exceptions thrown during test run.
Comment #8
sunNow also verifying HTML output after successful form submission, which can lead to dynamic page callbacks.
Comment #9
sunBetter title?
Comment #11
gregglesSeems like a great idea. I've tested out the project mentioned in comment #1. I'm not sure if you've used it, but basically it did a three step process:
This works reasonably well but doesn't take into account that step 2 can create new pages which are likely to be the source of XSS. Solving that problem in this work would be great.
@sun - it's not clear if you feel your current patch is ready for review/testing. Please state that when you are ready so others can provide input.
Comment #12
sunThe patch works reasonably well already. The tests failed for real -- because of possible XSS attacks or double-escaped form values in the output. :)
Not contained yet and only minimally prepared is recursion into forms on redirected pages, i.e. to test multi-step forms, but also sanitation of submitted form values on dynamic paths. For that, we have to abstract the logic that tries to find a form and attack it into a new helper method, so as to be able to call it recursively from itself. On subsequent redirection pages, we likely want to use menu_get_item($this->url) to figure out the $form_id on that page, and/or whether we want to add that page resp. $path to $this->page_callbacks for final XSS assertions afterwards.
Current docs are rudimentary, too...
I'm not sure whether I'll be able to work on this soon. Would be lovely to see someone else to take this up.
Comment #13
sunAlthough badly needed, this is D8 material according to the rules (I had to learn today). It may be backported at a later point in time (though that's unlikely).
Comment #14
jhedstromComment #15
dawehnerThis is test only code and so can be done at any point.