Form handlers assume $_POST elements will be strings

[tabacco]

I'm running Wordpress 3.5.1 on PHP 5.3.

This example is from lines 50-53 of wp-comments-post.php:

$comment_author       = ( isset($_POST['author']) )  ? trim(strip_tags($_POST['author'])) : null;
$comment_author_email = ( isset($_POST['email']) )   ? trim($_POST['email']) : null;
$comment_author_url   = ( isset($_POST['url']) )     ? trim($_POST['url']) : null;
$comment_content      = ( isset($_POST['comment']) ) ? trim($_POST['comment']) : null;

The issue is that If your post data contains something like:

author[]=foo&author[]=bar

or

comment[]=foo

Then the corresponding values in $_POST will be arrays, not strings, causing an E_WARNING.

There seem to be a number of other places where $_POST data is passed directly to PHP string functions as well, causing potential warnings. These should be handled gracefully by checking the type of the element being grabbed from $_POST first:

$comment_author       = ( isset($_POST['author']) && is_string($_POST['author']) )   ? trim(strip_tags($_POST['author'])) : null;
$comment_author_email = ( isset($_POST['email']) && is_string($_POST['email']) )     ? trim($_POST['email']) : null;
$comment_author_url   = ( isset($_POST['url']) && is_string($_POST['url']) )         ? trim($_POST['url']) : null;
$comment_content      = ( isset($_POST['comment']) && is_string($_POST['comment']) ) ? trim($_POST['comment']) : null;

The fact that WP doesn't expect an array in $_POST['author'] isn't a bug in itself, since comments can only have a single author. It seems to me that the bug lies in whatever code generates author[]=foo&author[]=bar.

This is an old debate, though: how far should we go in preventing notices?

No code generates it, but that doesn't mean it can't happen with fuzzers or pen testers. And in this case it's not a notice, it's a warning. You can make the case for not worrying about notices, but I do think warnings should be protected against.

So, what would be the practical benefit if people running fuzzers wouldn't get a warning?

The whole reason attackers use fuzzers is to find unhandled warnings and errors they can exploit. Not properly checking your inputs is a vulnerability waiting to happen. Just because it's not exploitable in this case doesn't mean it isn't somewhere else in the WP code base, or that it won't be in a future update.