1.7.1 Comment bug

I have just found a bug in the comments processing in version 1.7.1. If you post a comment as an author you are required to send {AUTHNAME}_{PASSWORD} for the comment to succeed. The { brackets are not necessary but the underscore is.

The problem is that the password is displayed in the comment preview and also in the posted comment. To fix this change the following lines (around 335 in gm-comment.cgi ]

Code:

my %newComment = ();
$newComment{‘name’} = $IN{‘newcommentauthor’};

to ]

Code:

my %newComment = ();
my @authname = split(/_/,$IN{‘newcommentauthor’});
$newComment{‘name’} = $authname[0];

and around line 414:

Code:

my %comment = ();
$comment{‘name’} = $IN{‘newcommentauthor’};

to

Code:

my %comment = ();
my @authname = split(/_/,$IN{‘newcommentauthor’});
$comment{‘name’} = $authname[0];

 

I tested for that specific issue prior to releasing 1.7.1. I was wondering if you have author protection enabled?

If you don’t, it won’t check your password and it will just post it. Author protection should not be required unless its enabled.

I will look into this. Do you have any other comment related features enabled? Perhaps there is a configuration I missed testing.

Author check is turned on. The rest of the relevant settings are as follows. I have allow karma or comment posting = comments only, order of comments = ascending, can post in archives = no, comment on by default = yes, html allowed = no, auto link = yes, passwd protect = yes, and the new comment links, pharse and force preview are all set.

I’ll look into that for sure. In the gm-comments.cgi, gm_authorcheck is called before either preview or post, and it modifies the authorname variable to basically what you posted.

I know this may seem silly, but I was wondering if you tried to post a without the password to see if it still enforces it? I think you mentioned changing the regex in gm_authorcheck, I wonder if that could have made a difference?

Yes i did check posting first as the author name and it was correctly blocked. Also i have the simple mod in place that I did for the previous author check issue you mention. I still believe that is valid as I wanted to post as “Pete” my username. Without my previuous fix I couldnt post as “Pete Finnigan” or “Pete Smith”. i.e. it blocked comments by people who were not users. I tried to post as Pete and it blocked me as I didnt supply the password, i then posted as PETE_PASSWORD and it worked but printed my password as well in the comment, hence the fix I posted above.

Ok, I believe I have this figured out. When this line was changed:

Code:

if ($IN{‘newcommentauthor’} =~ m/$a_name/i) {

to:

Code:

if ($IN{‘newcommentauthor’} =~ m/^$a_name$/i) {

I modified the logic to find an exact match, but an authorname_password, isn’t an exact match.

For example “Pete” is an exact match but “Pete_password” isn’t, so it doesn’t recognize it as a protect name, and it allows posting as usual.

You solution works, but I was banging my head against the wall trying to figure this out.

Thanks for your patience on this, as you have rightly pointed out my two fixes allow comments to be posted, they recognise only true usersnames but I have stopped the author password protect from working. Therefore the solution needs to be modified. The first fix that recognises only authr names needs to be changed to use the split on underscore first, i.e. check just the name part and then use the name_password to complete the author check. This would then mean that my fix posted in this thread is no longer necessary as the original author protect does not print the password. I will have a look at the code tomorrow and test it and feedback a solution here. And clarification!, sorry for the confusion.

The only issue I can think of is if there is an author with an underscore in their name, but this is easy to address by just splitting on ‘_’, then gettting last element (presumed password) and joining any remaining elements together with ‘_’.

I was delighted to find that you can do ‘split( ‘=’, $foo, 2) and it will split on the first = sign, but leave any others. I use it in the code to get name=value pairs from files. I just wish there was a way to tell it to work from back of string, short of reversing the string, splitting, then re-reversing both strings.

— Update —
Ok, after a quick trip to perlmonks.org, I found that the reverse thing would be easy:

Code:

my ($password, $authorname) = split( ‘_’, reverse( $lib ), 2);
$password = reverse( $password );
$authorname = reverse( $authorname );

— Update Again —
I have to stay away from perlmonks. But I did learn something new, I haven’t use ‘map’ much, but maybe I should:

Code:

my ($password, $authorname) = map {$_=reverse($_)} split( ‘_’, reverse( $lib ), 2);

Does it in one line, which would need at least one comment.

I finally got finished with the comment author check bug. I have included the original bug fix for the author name to ensure that the author check only picks up genuine author names. I have also removed the two small fixes described at the beginning of this thread as reported further up these are not necessary.

I have tested:-

user = genuine authname / no password – stops the post
user = genuine authname / wrong pwd – posts auth_wrongpwd – this is fine
user = genuine authname / correct password – posts fine – although i will discuss a slight issue at the end
user = authname not a gm user but is similar – i.e. my user is Pete and i tried Pete Finnigan – works fine

The case where we have a genuine author has a slight issue. If, like me you have not filled in the email, website etc then when posting as a genuine author there is no hyperlink created for the authorname even if one was entered in the commnet creation form. I think this is valid behaviour as its better to use the built in homepage for a real author rather than a supplied one. beware of this issue if you post as an author.

I have removed the two fixes at the beginning of this thread and also modified the function gm_authorcheck. The fix can be made by copying this complete function into gm-comments.cgi and deleting the old one. Here is the code:

Code:

## GM Author Check
# Args: ?
# Will check that the commenters name does not contain an author name. If it does
# will check that the comment author string is “AUTHOR_PASSWORD” and use the password
# to verify the identity of the author.
sub gm_authorcheck {my ($authorname) = “NULL”;
my ($password) = “NULL”;
## so that preview works, using org var, since we need to keep the ‘Name_Pw’
# note that we are _always_ setting this so that if we preview, a valid value
# goest into the commentauthorname isn’t a gm author …
$IN{‘newcommentauthor_org’} = $IN{‘newcommentauthor’};my $gmauthors = Gm_Storage::getAuthors( errHandler=>\&Gm_Web::displayUserErrorExit );foreach my $author ( keys( %$gmauthors ) ) {
my $a_name = $gmauthors->{$author}{‘author’};
my $a_password = $gmauthors->{$author}{‘password’};
my $a_email = $gmauthors->{$author}{’email’};
my $a_homepage = $gmauthors->{$author}{‘homepage’};

# because the AUTHOR_PASSWORD comes in as a single string and is compared with the list of
# blog authors we need to extract the name first here to check with the GM names to ensure
# that the name passed with a _PASSWORD is a correct author name. We could split the name
# and password in one line using map as described by Coldstone on the forum but I will use
# the long hand version here as its easier to read. We need to reverse the string to ensure
# that we still extract the name and password even if the name includes an underscore.

if ( $IN{‘newcommentauthor’} =~ m/_/g )
{
($password, $authorname) = split( ‘_’, reverse( $IN{‘newcommentauthor’} ), 2);
$password = reverse( $password );
$authorname = reverse( $authorname );
}
else
{
$authorname = $IN{‘newcommentauthor’};
$password = “ImPossssssiblePassword”;
}
# we are doing a regex to get a match since we don’t want to be fooled by leading spaces (or case)
if ($authorname =~ m/^$a_name$/i) {
## We got an athor name match, lets check the pw
# the pw will be attached to the endof the author name, seperated by ‘_’
my $commentpassword = crypt($password, $password);

## hmm, we are checking if the password matches both the plain and the crypted, because of Alice …
if ( ($a_password eq $commentpassword) || ($a_password eq $password) ){
$IN{‘newcommentauthor’} = $a_name;
$IN{‘newcommenthomepage’} = $a_homepage;
$IN{‘newcommentemail’}= $a_email;
} else {
println( Gm_Web::createRequestHeader() );
println( Gm_Web::createUserError(‘You cannot use that name to ‘.
‘post a comment without the correct password. Please pick ‘.
‘another name or check your password.’ ));

## Good to let someone know that people are trying to impersonate
Gm_Core::writeToCplog( “<B><FONT COLOR=\”#dd0000\”>Invalid Password commenting on entry “.
“#$IN{‘newcommententrynumber’}:</font></B> $IN{‘newcommentauthor’} “,
\&Gm_Web::displayUserErrorExit );
exit;
}
}
}
}

Sorry for all the confusion with these comments issues.

 

Related Posts