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.

CommentFileSizeAuthor
#18 1647440-filter-tips-check-valid-format-d7-18.patch3.12 KBgreggles
#13 1647440-filter-tips-check-valid-format-d7-13.patch3.16 KBchrisrockwell
#11 1647440-filter-tips-check-valid-format-d7-9.patch3.22 KBchrisrockwell
#9 1647440-filter-tips-check-valid-format-d7-#9.patch3.16 KBchrisrockwell
#5 1647440-filter-tips-check-valid-format-d7.patch1.51 KBchrisrockwell
#2 1647440-filter-tips-check-valid-format.patch3.89 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue tags: +Needs backport to D6, +Needs backport to D7

This should be backportable to both D6 and D7.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format.patch3.89 KB
ramlev’s picture

ramlev commented
Status: Needs review » Reviewed & tested by the community

Tested the patch in #2, works like a charm, and filter tests passed.

catch’s picture

catch
he/him
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Much nicer, thanks! Committed/pushed to 8.x, moving to 7.x for backport.

chrisrockwell’s picture

chrisrockwell commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format-d7.patch1.51 KB

Attempt to backport patch to d7

ben.bunk’s picture

ben.bunk commented
Status: Needs review » Reviewed & tested by the community

The D7 version works as expected. The patch could be easily rerolled to include the test cases from Dave Reid.

chrisrockwell’s picture

chrisrockwell commented

I'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.

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Needs work

Sounds 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.

chrisrockwell’s picture

chrisrockwell commented
Status: Needs work » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format-d7-#9.patch3.16 KB

2nd attempt. Thanks for the direction @David_Rothstein.

Status: Needs review » Needs work

The last submitted patch, 1647440-filter-tips-check-valid-format-d7-#9.patch, failed testing.

chrisrockwell’s picture

chrisrockwell commented
Status: Needs work » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format-d7-9.patch3.22 KB

No '#' in filename?

Status: Needs review » Needs work

The last submitted patch, 1647440-filter-tips-check-valid-format-d7-9.patch, failed testing.

chrisrockwell’s picture

chrisrockwell commented
Status: Needs work » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format-d7-13.patch3.16 KB

I'll figure it out.

Dave Reid queued 13: 1647440-filter-tips-check-valid-format-d7-13.patch for re-testing.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Issue summary: View changes
Issue tags: +Security improvements

Status: Needs review » Needs work

The last submitted patch, 13: 1647440-filter-tips-check-valid-format-d7-13.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
commented

From 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.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 1647440-filter-tips-check-valid-format-d7-18.patch3.12 KB

Let's try this.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

To be more clear: The test hunk wasn't applying, so I just manually re-rolled it.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

David_Rothstein commented
Version: 7.x-dev » 6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +7.40 release notes

Committed 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.

  • David_Rothstein committed 00206a4 on 7.x 
    Issue #1647440 by chrisrockwell, Dave Reid, greggles: Fix PHP notice if...
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I 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.

Status: Patch (to be ported) » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.