There are two
bugs outstanding that I can see as
active and potentially
bothersome. Both of them have "
exploits" if you will. The first
bug is with the
vote opcode.
Vote is an incredibly simple opcode, with very few checks to see whether or not you can vote on a
valid node. There are many
examples of nodes that you can't normally vote on, but that we only block through the
interface:
I found these bugs auditing over the code (yay open source), and anotherone says that they are known about. This is a fairly bad practice, but it's not so bad seeing as the bugs don't hurt people much, and this is a controlled community.
An example of the exploit is at
JayBonci's voting booth. If this becomes at all a
problem, this node is going to be
history fast. Gods: feel free to push this one
down in
flaming ruins if need be. That
edevdoc allows you to test that
theory, given the parent e2node id, and the
writeup id. With it, you can end up with
situations like:
{ marked for destruction } Reputation: 1 C?
The
solution to this
problem is in the
vote opcode:
We aren't checking for two things: whether or not it is on
node row, or whether or not it is a "
protected" writeup title.
Right now the code is:
my @params = $query->param;
use Everything::Experience;
foreach (@params) {
next unless /^vote\_\_(.*)$/;
my $N = $1;
my $val = $query->param($_);
getRef($N);
next if $$N{author_user} == getId($USER);
next unless ($val == 1 or $val == -1);
$query->param('test', $val);
castVote(getId($N), $USER, $val);
}
I think it should be something like:
my @params = $query->param;
#Get the id of node row once and hang on to it
my $nrid = getId(getNode('node row', 'superdoc'));
use Everything::Experience;
foreach (@params) {
next unless /^vote\_\_(.*)$/;
my $N = $1;
my $val = $query->param($_);
getRef($N);
my $nid = getId($N)
#very similar to the way we do it in voteit
my $kuid = $DB->sqlSelect('linkedby_user', 'weblog', "weblog_id=$nrid and to_node=$nid") || 0;
next if $kuid;
next if $$N{author_user} == getId($USER);
next unless ($val == 1 or $val == -1);
$query->param('test', $val);
castVote(getId($N), $USER, $val);
}
To be honest, I can't figure out how to stop the
voting from taking place on
E2 nuke requests, without doing an $$N(title) eq "All of restricted titles". Comments here would be
welcome.
The other bug is in the
Cool opcode. A weird and twitchy explanation would be in
Jurph's writeup under
Everything Cheat Codes. Not a pretty
situation, but it probably has to do with some kind of odd
race condition he's trying to
exploit. No matter.. we can shut down his (and other potential
abusers of the system) hopes' of getting "infinite cools". What seems to be the issue in my mind is that the cool operation is not at all atomic. At any point you could break the operation and still come out ahead...
The cool opcode looks something like this right now:
use Everything::Experience;
adjustExp($$COOL{author_user}, 10);
my $V = getVars($$COOL{author_user});
unless($$V{no_coolnotification}) {
my $P = getNode($$COOL{parent_e2node});
$DB->sqlInsert('message',
{msgtext => "\$$USER{title}\ just cooled your writeup on \$$P{title}\, baby",
author_user => getId(getNode('Cool Man Eddie', 'user')),
for_user => $$COOL{author_user}});
}
$DB->sqlInsert('coolwriteups', {coolwriteups_id => getId($COOL), cooledby_user => getId($USER)});
$$COOL{cooled} = 1;
updateNode($COOL, -1);
$$VARS{cools}--;
'';
But we could change it around a little, so that they lose the cool before we do anything, thus not breaking the system, but potentially causing partial cools: The XP, but no CME message, etc. Like this:
use Everything::Experience;
#There really isn't a place where one can fail, so subtract the cool here instead.
$$VARS(cools)--;
adjustExp($$COOL{author_user}, 10);
my $V = getVars($$COOL{author_user});
unless($$V{no_coolnotification}) {
my $P = getNode($$COOL{parent_e2node});
$DB->sqlInsert('message',
{msgtext => "\$$USER{title}\ just cooled your writeup on \$$P{title}\, baby",
author_user => getId(getNode('Cool Man Eddie', 'user')),
for_user => $$COOL{author_user}});
}
$DB->sqlInsert('coolwriteups', {coolwriteups_id => getId($COOL), cooledby_user => getId($USER)});
$$COOL{cooled} = 1;
updateNode($COOL, -1);
#This is going up a few lines
#$$VARS{cools}--;
'';
The cool opcode still isn't that atomic, but it now fall on the side of the system, and not the potential abuser. CastVote doesn't seem to be affected by such, as it seems slightly more atomic.
Comments, as always, are welcome.
N-Wing says:
cool and
vote opcodes fixed, except for voting on "special" e2nodes (that isn't a really big deal, and it should be fixed in a better way than checking all the titles anyway)