@dpnserver,
I spent the afternoon trying to debug the issue you reported with Snapshot and this Anti CSRF plugins. Like your posts to this thread when I try to save any part of Snapshot even something in Snapshots > Settings, which are all simple post forms, I get the generic error screen from Anti CSRF.
So being a plugin developer myself I went a little deeper into the Anti CSRF plugin code. Can’t say I like the way this plugin works. Where to start. Hopefully the plugin developer can read this and not just ignore the post.
1. The logic for validating the ‘bawac_force_nonce’ is not correct. See line 49 of the plugin:
if( count( $_REQUEST ) && ( !isset( $_REQUEST['bawac_force_nonce'] ) || !wp_verify_nonce( $_REQUEST['bawac_force_nonce'], '__BAW__' ) ) )
The problem is not the verify but that you are checking the _REQUEST super global. The root of the issue is that you are setting both a $_POST and $_GET ‘bawac_force_nonce’ instance. And get this they have different values. See next points.
2. I noticed in the admin ‘bawac_force_nonce_init’ function at line 57 you start caching the content. In the footer function ‘bawac_force_nonce_foot’ at line 89 you then call ‘ob_get_contents’ to get all the cached page content. Sorry but this is just wrong. You may think setting the priority to 999 will mean your footer function is called last but it is not a guarantee. I can setup a footer function with priority or 10000 and then add some malicious links and JS.
3. Creating duplication of ‘bawac_force_nonce’ form variable. And they have two different values. As I mentioned in point #1 you are crestings both a _POST and _GET variable ‘bawac_force_nonce’. At line 91 you have the following code
wp_nonce_field( '__BAW__' . date( 'a' ), 'bawac_force_nonce', false, false )
Notice the date(‘a’) element. Not sure why this is even here. why would you think you need to add a unique element to a function which in itself produces unique output. As a result in the actual form you end up with two different values
<form method="post" action="?bawac_force_nonce=9b4df144f1&page=snapshots_settings_panel">
<input id="bawac_force_nonce" type="hidden" value="1bac9b78dc" name="bawac_force_nonce">
<input type="hidden" value="snapshots_settings_panel" name="page">
As I mentioned in point #1 you are checking only _REQUEST. Since this then depends on the php.ini settings for ‘variables_order’ ‘https://us3.php.net/manual/en/ini.core.php#ini.variables-order you don’t have control of if _REQUEST is taken from _GET or _POST. IT may work on one PHP setup but not another.
A proper solution would be to check the _SERVER[‘REQUEST_METHOD’] then check the appropriate ‘bawac_force_nonce’ global. Then use that to test in your if statement.
if ($_SERVER[‘REQUEST_METHOD’] == “POST”) {
$verify_ret = wp_verify_nonce( $_POST[‘bawac_force_nonce’], ‘__BAW__’ . date( ‘a’ ) );
} else {
$verify_ret = wp_verify_nonce( $_GET[‘bawac_force_nonce’], ‘__BAW__’ );
}
if(count( $_REQUEST ) && ( !isset( $_REQUEST[‘bawac_force_nonce’] ) || !$verify_ret ) )
Also I don’t really see the point of checking the count of _REQUEST items. who cares. So if it is empty this is cause for an error? Not logical.
4. And last point back to your footer. At line 91 you are not only adding your own nonce var which I understand. But why do you think it valid to add the ‘page’ reference? Again this override (potentially) the page var in the form action URL which is the important one. From what I can tell you logic is not effecting or changing the page value which is great. But again it causes duplicate global vars. You don’t own the page the plugin who create the page does. It has rights to the ‘page’ element in the forms not your plugin.
