Make WordPress Core

Opened 9 years ago

Last modified 2 years ago

#10551 closed defect (bug)

wp_die() triggers block when using ModSecurity Core Rules — at Version 15

Reported by: cstrosser Owned by: westi
Milestone: 4.1 Priority: low
Severity: normal Version: 2.7
Component: Security Keywords: westi-likes has-patch needs-docs
Focuses: Cc:

Description (last modified by nacin)

wp_die() causes ModSecurity (using Core Rules), a commonly used Apache plugin, to throw a 406 error, blocking the message from displaying.

This seems to be triggered by the fact that a 500 error is thrown. I went and changed the default status code to 401 (Unauthorized) and it worked like a charm. I just wonder if there is a better fix for this issue... or simply a better status code to use.

Perhaps making a group of functions to make the error codes more focused.


wp_die_auth( __('You do not have sufficient permissions to access this page.') );

 * Exits WordPress with Unauthorized status code.
 * @see wp_die()
function wp_die_auth($message, $title = '') {
	wp_die( $message, $title, 401 );

Change History (16)

#1 @cstrosser
9 years ago

  • Summary changed from wp_die() triggers block when using ModSecurity core rule set to wp_die() triggers block when using ModSecurity Core Rules

#2 @westi
9 years ago

  • Keywords reporter-feedback added; ModSecurity wp_die 406 error removed
  • Owner changed from ryan to westi
  • Status changed from new to accepted
  • Version set to 2.8.3

Looking at http://www.owasp.org/index.php/Category:OWASP_ModSecurity_Core_Rule_Set_Project#tab=About these rules are only Alpha quality.

Do you have more info on what the rule is trying to prevent?

#3 @dd32
9 years ago

I have a feeling its recently released, see http://blog.modsecurity.org/

If i recall correctly, Its due to some security people disliking 500 errors for web app error handling, and wanting people to use the 40x error set. But theres not a 400 error code for all cases either.. IMO, the rule is pointless.

#4 @Denis-de-Bernardy
9 years ago

  • Milestone changed from Unassigned to 3.0

#5 @nacin
8 years ago

See also #11286.

I'm willing to take ownership of this and go through wp_die references and add response codes. I imagine that would mean creating some shortcut wp_die functions different requests, i.e. authentication, forbidden, not found, internal error, (401, 403, 404, 500) etc.

#6 @miqrogroove
8 years ago

Status 401 is not appropriate, unless someone has been adding a lot of features I didn't know about.

#8 @aaroncampbell
8 years ago

Please see also #12341 and #12529. Here's why I don't like 500 errors (pasted from #12529):

Part of the reason I want to do this, is that I've noticed that automated systems trying to break a site seem to see a 500 error as a clue that they are getting closer to their goal. They seem to pound a lot harder on the scripts that return 500 errors, so I'd like to clean these up and only return them when they're actually appropriate.

Currently wp_die is used 278 times in 86 files. However, we only specify an HTTP response code 3 times, which means 99% of these calls return a 500 status code.

#9 @aaroncampbell
8 years ago

  • Cc aaron@… added

#10 @nacin
8 years ago

  • Keywords early added; reporter-feedback removed
  • Milestone changed from 3.0 to 3.1

Let's do this early 3.1.

#11 @nacin
8 years ago

  • Keywords early removed
  • Milestone changed from Awaiting Triage to Future Release

#12 @lloydbudd
8 years ago

Here is another symptom reported by a customer:

"On a blackberry bold if I try to submit a comment that is empty the blackberry displays a message saying that a 500 error was generated instead of the page with the message that the comment was empty. The blackberry stops the page from loading which leaves the user confused."

#13 @westi
8 years ago

  • Keywords needs-patch 3.2-early added

#14 @solarissmoke
7 years ago

  • Keywords dev-feedback added

Here's a patch which proposes a bunch of new functions to call wp_die with different status codes. Meanwhile the default response is changed to 200.

I'm thinking that we need consensus on this first. Then it is just a case of sifting through all existing wp_die() calls and changing them accordingly.

#15 @nacin
7 years ago

  • Description modified (diff)

Don't think we should change the default.

Note: See TracTickets for help on using tickets.