| @@ -174,18 +174,19 @@ | ||
| 174 | 174 | private function gather_request_data() { |
| 175 | 175 | $this->haystack = null; |
| 176 | 176 | |
| 177 | 177 | $this->request_data = array( |
| 178 | - 'uri' => isset( $_SERVER['REQUEST_URI'] ) ? sanitize_text_field( wp_unslash( $_SERVER['REQUEST_URI'] ) ) : '', | |
| 179 | - 'query_string'=> isset( $_SERVER['QUERY_STRING'] ) ? sanitize_text_field( wp_unslash( $_SERVER['QUERY_STRING'] ) ) : '', | |
| 180 | 178 | /* |
| 181 | 179 | * Copies that keep the percent encoding, used only as the haystack |
| 182 | 180 | * of the pattern checks and never logged, printed or stored. |
| 183 | 181 | * |
| 184 | 182 | * They exist because sanitize_text_field() deletes every %XX |
| 185 | - * sequence instead of decoding it: the copies above are the payload | |
| 183 | + * sequence instead of decoding it: a sanitized copy is the payload | |
| 186 | 184 | * with the evidence removed, so an encoded attack was invisible to |
| 187 | - * every rule that reads them. Measured on 22 aug 2026 against 2.9.8, | |
| 185 | + * every rule that read one. Two such copies (uri and query_string) | |
| 186 | + * used to sit above these; nothing read them, because the log takes | |
| 187 | + * loggable_uri(), so they were removed and the next reader does not | |
| 188 | + * copy the pattern. Measured on 22 aug 2026 against 2.9.8, | |
| 188 | 189 | * ?x=%3Cscript%3E, javascript%3A, php%3A%2F%2F and GLOBALS%5B all |
| 189 | 190 | * reached the checks as harmless text and went straight through. |
| 190 | 191 | * |
| 191 | 192 | * No sanitizer is applied, and that is the point: every one of them |
| @@ -191,13 +192,16 @@ | ||
| 191 | 192 | * No sanitizer is applied, and that is the point: every one of them |
| 192 | 193 | * destroys exactly what has to be matched. sanitize_text_field() |
| 193 | 194 | * deletes the %XX sequences and strips tags. esc_url_raw() is worse |
| 194 | 195 | * here: measured on 22 aug 2026, it returns an empty string for a |
| 195 | - * query that carries an unencoded :// , which is precisely the | |
| 196 | - * remote inclusion shape, so it would blind the firewall instead of | |
| 197 | - * arming it. These two values are never echoed, never stored and | |
| 198 | - * never reach a query; they are the haystack of preg_match() and | |
| 199 | - * nothing else. | |
| 196 | + * query read on its own that carries an unencoded :// , which is | |
| 197 | + * precisely the remote inclusion shape, and over a whole target it | |
| 198 | + * drops < > and quotes, which are what the rules look for, so it | |
| 199 | + * would blind the firewall instead of arming it. (For anything that | |
| 200 | + * is not a pattern check, the reader of the request target is | |
| 201 | + * Vigilante_IP_Utils::request_uri().) These two values are never | |
| 202 | + * echoed, never stored and never reach a query; they are the | |
| 203 | + * haystack of preg_match() and nothing else. | |
| 200 | 204 | */ |
| 201 | 205 | // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- inspection buffer for the pattern checks, see the note above. Sanitizing it is what hid the attacks. Never output, stored nor queried. |
| 202 | 206 | 'uri_raw' => isset( $_SERVER['REQUEST_URI'] ) ? wp_unslash( $_SERVER['REQUEST_URI'] ) : '', |
| 203 | 207 | // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- same as uri_raw. |
| @@ -1074,11 +1078,26 @@ | ||
| 1074 | 1078 | return; |
| 1075 | 1079 | } |
| 1076 | 1080 | |
| 1077 | 1081 | $method = strtoupper( $this->request_data['method'] ); |
| 1078 | - $allowed_methods = isset( $this->options['allowed_http_methods'] ) | |
| 1079 | - ? $this->options['allowed_http_methods'] | |
| 1080 | - : array( 'GET', 'POST', 'HEAD', 'OPTIONS', 'PUT', 'PATCH', 'DELETE' ); | |
| 1082 | + /* | |
| 1083 | + * An empty list here means "nothing was stored", not "the site wants | |
| 1084 | + * none". These four lists have no field on any screen, so nobody can | |
| 1085 | + * empty them on purpose, and every version up to 3.0.0 blanked them in | |
| 1086 | + * the database whenever their tab was saved. Reading the settings used | |
| 1087 | + * to hide it by merging the shipped values over the top; since 3.0.1 a | |
| 1088 | + * stored empty list is respected, which is what makes unticking every | |
| 1089 | + * role work. The migration in vigilante.php removes the blanked keys | |
| 1090 | + * once, and this is the second line of defence: a list that arrives | |
| 1091 | + * empty by any other route falls back to what ships, instead of | |
| 1092 | + * blocking every HTTP method, hiding a protection that the screen says | |
| 1093 | + * is on, or cutting the public REST API. The list itself lives in | |
| 1094 | + * get_default_options(), read through list_or_shipped(). | |
| 1095 | + */ | |
| 1096 | + $allowed_methods = Vigilante_Settings::list_or_shipped( | |
| 1097 | + $this->options['allowed_http_methods'] ?? array(), | |
| 1098 | + array( 'firewall', 'allowed_http_methods' ) | |
| 1099 | + ); | |
| 1081 | 1100 | $allowed = array_map( 'strtoupper', $allowed_methods ); |
| 1082 | 1101 | |
| 1083 | 1102 | if ( ! in_array( $method, $allowed, true ) ) { |
| 1084 | 1103 | $this->block_request( |
