Enable Nonce Verification Sniff.

[peterwilsoncc]

Description of the Change

Reenable the WordPress.Security.NonceVerification.Missing to reduce the chance of inadvertent CSRF errors in our code.

Additionally, wp_verify_nonce() is configured as an unslashing and sanitizing function. This it for developer ease due to the way nonce values are defined in WordPress and how the value is compared within the function.

The sniff was apparently disabled because it can be difficult to work with. There are two approaches I've found that enforce the sniff quite reliably.

In instances the csrf token (nonce) is required

function my_csrf_suseptable_function() {
  if ( empty( $_GET['thenonce'] ) {
    return false;
  }

  if ( ! wp_verify_nonce( $_GET['thenonce'], 'my-action-name' ) ) {
    return false;
  }
  
  // current_user_can check followed by required functionality.
}

In instnaces the csrf token (nonce) is not required

function my_function_expecting_a_query_string_param() {
   /*
    * This function does not require a nonce.
    * 
    * The query string parameters in this function are used without a nonce to modify
    * the output displayed to the user according to the page they are on. It does not
    * accept user input for recording in the database so a nonce check is not required.
    */
  // phpcs:disable WordPress.Security.NonceVerification.Missing

  // Do functionality requiring query string parameters

  // phpcs:enable

  // Do other functionality.
}

Closes #38

How to test the Change

1. In a project with these sniffs applied, write the function

   function testing() {
      $input = isset( $_GET['input'] ) ? sanitize_text_field( wp_unslash( $_GET['input'] ) ) : '';
   }
   

2. Run ./vendor/bin/phpcs .
3. On this branch phpcs should alert the developer to the lack of nonce in the function.

Changelog Entry

Security - Enabled the WordPress.Security.NonceVerification.Missing sniff.

Credits

Props @peterwilsoncc, @dkotter.

Checklist:

• [x] I agree to follow this project's Code of Conduct.
• [ ] I have updated the documentation accordingly.
• [ ] I have added tests to cover my change.
• [ ] All new and existing tests pass.

[darylldoyle]

@peterwilsoncc, whilst I agree with re-enabling this sniff, I'm concerned that it'll lead to engineers getting stuck trying to shoehorn nonces into the codebase where they're not needed. It sounds like that was the reason it was initially removed.

I think if we are to re-enable this sniff it should be accompanied by documentation on the rules around ignoring it and the examples that you've included here. This documentation should live somewhere that it can be easily exposed to the entire company, potentially: https://10up.github.io/Engineering-Best-Practices/php/#nonces.

Thoughts?