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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

This should be backportable to both D6 and D7.

Dave Reid’s picture

Status: Active » Needs review
FileSize
3.89 KB
ramlev’s picture

Status: Needs review » Reviewed & tested by the community

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

catch’s picture

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

Status: Patch (to be ported) » Needs review
FileSize
1.51 KB

Attempt to backport patch to d7

ben.bunk’s picture

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

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

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

Status: Needs work » Needs review
FileSize
3.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

Status: Needs work » Needs review
FileSize
3.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

Status: Needs work » Needs review
FileSize
3.16 KB

I'll figure it out.

Dave Reid’s picture

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

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

Status: Needs work » Needs review
FileSize
3.12 KB

Let's try this.

greggles’s picture

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

Dave Reid’s picture

Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

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

greggles’s picture

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.