WordPress.org

Ready to get started?Download WordPress

Forums

[Plugin: AVH First Defense Against Spam] Bug in comment nonce (4 posts)

  1. shidouhikari
    Member
    Posted 4 years ago #

    Hello, I'd like to report a bug I found. I tried to register on your forum but didn't receive activation email...

    In my site, commenting works fine for posts, but somebody tried to comment on a page and was getting nonce check failed error.

    Comment wasn't even saved and I received them from email, so I tried to make it myself... but I was being accused of cheating too! I tried logged off and logged in, same error, WTF?!

    Since "Cheating huh?" means nothing, I searched plugin files for this string, and discovered it was indeed a nonce check error. It's in avh-fdas.public.php line 135.

    In the error page, I verified both variables being compared:
    $nonce: 958b547e4b
    $_POST['_avh_first_defense_against_spam']: 5d110c9f6d

    Yeah, something really wrong. Maybe nonce generator was bugged? It's in the same file, line 50. 'avh-first-defense-against-spam_' is equal in both, but when I tested post ID...

    $post: 24
    $post->ID: of course NULL
    $post_id: consequently NULL

    $post_id is used to create the nonce, so it's created with 'avh-first-defense-against-spam_'.NULL and then tested as 'avh-first-defense-against-spam_'.24, of course will fail.

    You test if $post is empty, and it's not, but you believe it's an object, and it's integer.

    As expected, for posts $post is an object, and nonce works as expected.

    My suggestion is, if $post isn't empty, also test if it's object, and even if it's array, and then try integer. IDK why this inconsistent behavior of WordPress, sad :( Maybe you should try custom page types too, they may behave differently.

    2 suggestion: I've received complains of ppl trying to enter my site and being blocked out, sometimes with 403 error. I suspect their IP is blacklisted in HoneyPot, it would be nice to block only commenting from IPs blacklisted for spam commenting.

    I myself only wanna block the whole site to harvesters, mail spammers and comment spammers make no harm visiting my site. Dynamic IP is also a problem, because bots may come harvest and pass the result to spammers, but somebody infected may blacklist an IP and then another person with that IP will be blocked, without any guilt.

    Error messages are also weak, "Cheating huh?" is also used by WordPress core and means nothing. As we saw, behaviors that could mean cheating can also mean a bug, and nobody likes to be accused of cheating when trying to comment on a site... A hacker for sure will be able as I was to find what that error really means, so there's no problem of clearly explaining what went wrong, with a polite text.

    I'd also like to be able to change default error messages without having to mess with plugin files. Maybe you could put these messages on a separated file, so that we can easily update the plugin and just copy our file back then.

    If it's possible, it would be nice to have a user friendly option, to set which blocking behavior should be taken for each kind of blacklist. Mail spam would allow full access to the site, but a theme message would complain that IP was blacklisted from spamming and say to stop. Comment spam would have access to the site but not be allowed to comment, and harvesters would get a wp_die explaining what happened.

    At least, it would be nice to have a IP blacklist and the one we have now, but whose IPs would be removed from the list 1 week after being added. I blacklist a lot of IPs, mostly flooders that are trashing my visits stats database, but also most of them flood me for some days and then go away. 1 week would be enough to block then, and then these IPs can leave the blacklist.

    Anyway, tnx for the great plugin, it helps a lot to keep spammers away :D

    http://wordpress.org/extend/plugins/avh-first-defense-against-spam/

  2. shidouhikari
    Member
    Posted 4 years ago #

    After the post I noted you are using global $post and not the action parameter. Well, $post should always be an object :)

    Investigating what could be making $post be an integer, I checked a few plugins I use and then noted that it was integer even before the action was called. I kept tracking it until I discovered it's a core bug, so I opened a ticket: http://core.trac.wordpress.org/ticket/13753

    It's really odd WordPress is making $post be an integer on pages, get_page() is being called when edit_page capability is tested.

    Anyway, really sorry for my misunderstood :P We plugins developers should be allowed to assume $post is an object when it's not empty. Unfortunately this variable is too much used and is very bug prone, and you were awarded with one! :P

    I'm gonna make a durty fix restoring $post in my theme until the bug is fixed, you should also do it to make sure all your users are safe. Let's hope They fix it still before 3.0 Final is released, but I won't be shocked if they throw it to 3.1, then to 3.2, then...

  3. shidouhikari
    Member
    Posted 4 years ago #

    Another note, your nonce is securily weak. Anybody knowing the post ID can generate the nonce, pass a parameter with it and break your security measure.

    I always like to use __FILE__ in my nonces, it's the full path of the file, so anybody wanting to hack the nonce would need to know where wp-content is in the server. Not impossible to discover, but makes it harder.

    You can also try the user login name if he's logged, or the session ID if not. Doing so, when the session expires the nonce is gone too, fair enough. It's problem is that it doesn't work when browser has cookie disabled.

  4. petervanderdoes
    Member
    Posted 4 years ago #

    First of all thanks for lengthy posts, really helpful.

    The nonce is generated with the Authentication Unique Keys and Salts you set in the wp-config.php file. If you don't change the default value WordPress will generate a new salt and store it in the DB for future usage. (see wp-include/pluggable.php 1326-1393)
    This is in WordPress 3.0 SVN.
    So chances of recreating the nonce should be pretty slim

Topic Closed

This topic has been closed to new replies.

About this Topic