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)
|
