If you got to mysite.com/filter/tips/invalid-format then you get the following PHP notices:
Notice: Undefined index: testtest in _filter_tips() (line 1013 of drupal7dev/modules/filter/filter.module).
Notice: Trying to get property of non-object in _filter_tips() (line 1017 of drupal7dev/modules/filter/filter.module).
Notice: Trying to get property of non-object in _filter_tips() (line 1018 of drupal7dev/modules/filter/filter.module).
Let's prevent this from happening.
Comments
Comment #1
Dave ReidThis should be backportable to both D6 and D7.
Comment #2
Dave ReidComment #3
ramlev CreditAttribution: ramlev commentedTested the patch in #2, works like a charm, and filter tests passed.
Comment #4
catchMuch nicer, thanks! Committed/pushed to 8.x, moving to 7.x for backport.
Comment #5
chrisrockwell CreditAttribution: chrisrockwell commentedAttempt to backport patch to d7
Comment #6
ben.bunk CreditAttribution: ben.bunk commentedThe D7 version works as expected. The patch could be easily rerolled to include the test cases from Dave Reid.
Comment #7
chrisrockwell CreditAttribution: chrisrockwell commentedI'm looking into how to do that; just not sure where I get the files that Dave Reid modified as they don't seem to be included in D7.
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedSounds like this needs work then, for the tests.
The files aren't the same in D7 and D8; you'd have to search the codebase for the relevant surrounding code in D7, but most likely it's all in modules/filter/filter.test.
Comment #9
chrisrockwell CreditAttribution: chrisrockwell commented2nd attempt. Thanks for the direction @David_Rothstein.
Comment #11
chrisrockwell CreditAttribution: chrisrockwell commentedNo '#' in filename?
Comment #13
chrisrockwell CreditAttribution: chrisrockwell commentedI'll figure it out.
Comment #15
Dave ReidComment #17
catchFrom reviewing 6.x, this may not be an issue there - https://api.drupal.org/api/drupal/modules!filter!filter.module/function/... does a database query rather than looking for an array index.
Comment #18
gregglesLet's try this.
Comment #19
gregglesTo be more clear: The test hunk wasn't applying, so I just manually re-rolled it.
Comment #20
Dave ReidComment #21
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedCommitted to 7.x - thanks!
Not sure if this really qualifies for the "Security improvements" tag since as far as I can see the user wouldn't have seen any information about the format on that page previously either?
I suspect there is some fix needed here in Drupal 6 too though, so I'm setting it to Drupal 6 based on the backport tag, but not really sure it makes sense to fix there at this point.
Comment #23
Dave ReidComment #24
gregglesI think the security risk is basically that many sites do not have errors turned to watchdog only and this notice is a way to reveal the path to the Drupal installation which might be useful to an attacker.
It's just a bit of information to help fingerprint a site and doesn't, by itself, present any direct risk. It's also a bit of information that should not be printed to the screen on properly secured sites. But...not all sites are properly secured.