- Development - lists.drupal.org

[drupal-devel] Release critical bugs for August 18, 2005
by drupal-devel＠drupal.org 18 Aug '05

18 Aug '05

/ bug reports email spam filtering does not "see" email addresses when they are NOT preceded by a word age: 30 weeks 4 days url: http://drupal.org/node/15654 / bug reports Error when anonymous user creates content age: 20 hours 50 sec url: http://drupal.org/node/29032 Reset user mail variables. age: 3 days 11 hours url: http://drupal.org/node/28868 Free Tagging broken on forums age: 1 week 18 hours url: http://drupal.org/node/28607 404 sites shown twice age: 2 weeks 2 days url: http://drupal.org/node/27990 js addLoadEvent not working. age: 2 weeks 4 days url: http://drupal.org/node/27884 Forum bug with drupal 462 age: 2 weeks 6 days url: http://drupal.org/node/27663 enable MySQL client side for UTF8 age: 4 weeks 2 days url: http://drupal.org/node/26990 Drupal 4.6.2 Doesn't allow login (IIS 5.1, PHP 5.0.4) assigned: bigeye age: 4 weeks 5 days url: http://drupal.org/node/26780 Problem in the 4.6.2 upgrade - node.module age: 6 weeks 2 days url: http://drupal.org/node/26342 postgresql autocomplete code age: 7 weeks 9 hours url: http://drupal.org/node/26027 Accidentally Deleting Forums Vocabulary in Categories Breaks Forums? age: 10 weeks 5 days url: http://drupal.org/node/24274 Taxonomy Multiple Hierarchy doesn't display children for all its parent age: 11 weeks 1 day url: http://drupal.org/node/24023 PG_QUERY ERROR - LOCK TABLES - IMPLEMENTATION FAULT. age: 13 weeks 3 days url: http://drupal.org/node/22911 database table "term_node" not updated when removing a term from a node under editing and submitting age: 18 weeks 2 days url: http://drupal.org/node/20315 Reassigning a node to a different term does not work properly age: 18 weeks 3 days url: http://drupal.org/node/20278 4.6 aggregator showing/not interpreting HTML tags age: 19 weeks 4 days url: http://drupal.org/node/19874 New submit error, non-admin user, postgresql database assigned: adrian age: 23 weeks 1 day url: http://drupal.org/node/18552 When I delete a user, any forum in which s/he has commented also disappears. assigned: Jill6q age: 26 weeks 14 hours url: http://drupal.org/node/17428 Change to sesssion.inc violates UID database constraint age: 30 weeks 4 days url: http://drupal.org/node/15666 forum.module problem with postgresql v7.4.1 age: 31 weeks 2 days url: http://drupal.org/node/15411 PHP 5 gives error when session table has no session assigned: adschar age: 31 weeks 2 days url: http://drupal.org/node/15399 Argument NOT must be type boolean age: 39 weeks 2 days url: http://drupal.org/node/12950 Comments in approval queue still show up in tracker age: 43 weeks 4 days url: http://drupal.org/node/11647 Twin problems with comments assigned: Junyor age: 44 weeks 6 days url: http://drupal.org/node/11366 Fatal error: Call to undefined function: drupal_set_html_head() age: 51 weeks 1 day url: http://drupal.org/node/10317 user error: Duplicate entry assigned: Kjartan age: 1 year 36 weeks url: http://drupal.org/node/4428 / feature requests Site Slogan Margin age: 1 week 4 days url: http://drupal.org/node/28382 Filter the title to allow HTML comments age: 3 weeks 6 days url: http://drupal.org/node/27205 Novel Module Creation Request age: 9 weeks 3 days url: http://drupal.org/node/24856 moving pages en masse age: 19 weeks 6 days url: http://drupal.org/node/19754 Taxonomy module should use nodeapi 'load' age: 23 weeks 13 hours url: http://drupal.org/node/18631 Auto PHP memory maximisation age: 25 weeks 2 days url: http://drupal.org/node/17663 Allow named anchors to work without specifying full path to node age: 1 year 38 weeks url: http://drupal.org/node/4213 / support requests blocks age: 8 weeks 2 days url: http://drupal.org/node/25378 Problem with Forums age: 31 weeks 5 days url: http://drupal.org/node/15299 / bug reports search over fields.inc fields conceptual error age: 41 weeks 6 days url: http://drupal.org/node/12259 fields.inc screws up with multible multiselect fields age: 41 weeks 6 days url: http://drupal.org/node/12257 / feature requests event.module support of repeating/recurring events? assigned: thehunmonkgroup age: 1 year 39 weeks url: http://drupal.org/node/4122 / bug reports Menu item not showing age: 45 weeks 5 days url: http://drupal.org/node/11210 / bug reports segmentation fault related to filestore age: 1 year 46 weeks url: http://drupal.org/node/3001 / bug reports file size bug age: 9 weeks 5 days url: http://drupal.org/node/24728 / bug reports gallery2 embeded bug with RC1 age: 8 hours 33 min url: http://drupal.org/node/29090 gallery bug - not integrated in drupal assigned: vidy age: 1 week 22 hours url: http://drupal.org/node/28598 gallery.module doesn't synch passwords when they're changed age: 2 weeks 6 days url: http://drupal.org/node/27702 Full screen slideshow in Gallery module is not working (erroring out on the wrong URL) age: 6 weeks 9 hours url: http://drupal.org/node/26479 Drupal session being logged out each time gallery module is called age: 7 weeks 6 days url: http://drupal.org/node/25614 Gallery error age: 16 weeks 2 days url: http://drupal.org/node/21290 Fatal Error: variable_get() age: 21 weeks 3 days url: http://drupal.org/node/19189 No Gallery Image Block age: 21 weeks 6 days url: http://drupal.org/node/19045 Mini applet can't find gallery at url / age: 22 weeks 4 days url: http://drupal.org/node/18785 Fatal error: Undefined class name 'galleryembed' age: 25 weeks 1 day url: http://drupal.org/node/17746 / support requests Can't Embed Gallery into Drupal age: 24 weeks 4 days url: http://drupal.org/node/18029 / bug reports Drupal-4.4-rc breaks registration setups and groups.module age: 1 year 22 weeks url: http://drupal.org/node/6392 / bug reports image module fails on dual site configuration age: 5 weeks 6 hours url: http://drupal.org/node/26657 Editing a previously uploaded and thumnailed image loses the thumbnail. age: 8 weeks 1 day url: http://drupal.org/node/25465 Cannot upload .jpgs age: 24 weeks 3 days url: http://drupal.org/node/18076 Image.Module does not know the correct filename age: 39 weeks 1 day url: http://drupal.org/node/13032 / support requests Images are there but not showing assigned: Stepfordtwit age: 3 weeks 13 hours url: http://drupal.org/node/27615 Image Module Permissions age: 6 weeks 3 days url: http://drupal.org/node/26280 path problems on upgrading... age: 15 weeks 3 days url: http://drupal.org/node/21725 / bug reports RSS "no element found at line 1" age: 6 weeks 5 days url: http://drupal.org/node/26185 Outdated help texts age: 1 year 2 weeks url: http://drupal.org/node/9692 Import doesn't like XML/RSS feeds with no Title element age: 1 year 11 weeks url: http://drupal.org/node/8128 / tasks move XML-parser into an inclde file. age: 8 weeks 1 day url: http://drupal.org/node/25439 / feature requests Notify.module ignores the true submission queue age: 1 year 42 weeks url: http://drupal.org/node/3765 / feature requests Internationalization support for PDF assigned: TheLibrarian age: 1 year 20 weeks url: http://drupal.org/node/6795 / bug reports Reminder email links don't work age: 7 weeks 5 days url: http://drupal.org/node/25677 / bug reports access control problems age: 3 weeks 5 days url: http://drupal.org/node/27273 Download link is wrong age: 8 weeks 2 days url: http://drupal.org/node/25363 Projects not listed and page not complete age: 11 weeks 2 days url: http://drupal.org/node/23956 Date for latest is incorrect age: 18 weeks 1 hour url: http://drupal.org/node/20485 Invalid link to release files age: 25 weeks 3 days url: http://drupal.org/node/17625 Node body not complete w/o releases age: 27 weeks 6 days url: http://drupal.org/node/16733 blank page after install age: 28 weeks 1 day url: http://drupal.org/node/16575 / support requests Help with everything - nothing seems to work in the latest build age: 38 weeks 2 days url: http://drupal.org/node/13310 / bug reports Can't get the inline tag to show series age: 14 weeks 1 day url: http://drupal.org/node/22520 / feature requests Request to generate email for all content changes/additions, including comments age: 4 weeks 4 days url: http://drupal.org/node/26824 / bug reports javascrip downloads square.gif for every category age: 26 weeks 1 day url: http://drupal.org/node/17366 / bug reports Link Checking? age: 4 days 1 hour url: http://drupal.org/node/28841

1 0

[drupal-devel] [bug] drupal base and E_NOTICE
by Thomas Ilsche 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/28540 Post a follow up: http://drupal.org/project/comments/add/28540 Project: Drupal Version: 4.6.0 Component: base system Category: bug reports Priority: minor Assigned to: Anonymous Reported by: hlslaughter Updated by: Thomas Ilsche Status: patch (code needs review) This is my personal opinion, I do not like any @ usage. Those errors are there for a reason. They help finding real bugs. For E_NOTICE epsecially for typos, imagine those index is not just missing, its simply mistyped but hardly used... I prefer to initialize variables as much as possible. If it really is necesary isset. The problem with if($var['index']) is that it can have different meanings: Check if the index exist (bogus for 0, ''...) Check if $var['index'] is actually 0 / '' / NULL Use a real boolean value, Any combination of this. when someone reads this concise form it is hardly possible to tell what meaning it should have. This is why I personally prefer more detailled checks using isset,==(=),is_null etc. And as mentioned already they do help finding typos/bugs. If $var['index'] is supposed to exist and you get a notice then you know there is something wrong. Thomas Ilsche Previous comments: ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:19:33 +0000 : hlslaughter ok, instead of whining in the forums, i'll submit a ticket. i'm sure i'm not the first person to bring this up, and i apologize in advance if this issue is already decided/closed. but my issue is this: would it be possible to review the core modules and clean up code to the point that it would be feasible to run drupal with notices turned on (at least in a dev environment)? i think it would be pretty trivial to clean most/all of these errors from the code base. the cost of this effort would be relatively teeny, and the benefits would be great. admins would no longer panic if they happen to see these messages; with an option added to the settings screen, developers could turn on notices and would write cleaner code, which would probably result in fewer bugs in 3rd party modules; etc... you know the routine 8*) after turning on E_NOTICE in common.inc, i see the following notice messages when accessing the homepage of my site: occurences - message ----V--------------V-------- 12 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 114. 10 notice: Undefined variable: teaser in /var/www/html/modules/opt/attachment.module on line 37. 6 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 120. 6 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 176. 4 notice: Undefined variable: rows in /var/www/html/modules/opt/attachment.module on line 352. 4 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 123. 4 notice: Undefined variable: attachments in /var/www/html/modules/opt/attachment.module on line 279. 1 notice: Undefined variable: node in /var/www/html/modules/node.module on line 1950. 1 notice: Undefined offset: 0 in /var/www/html/includes/pager.inc on line 59. 1 notice: Undefined index: op in /var/www/html/modules/node.module on line 1636. 1 notice: Undefined index: node.taxonomy in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: node.picture in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.title in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.site_slogan in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.mission in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.help in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: from in /var/www/html/includes/pager.inc on line 53. 1 notice: Undefined index: footer.message in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: footer.blocks in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: edit in /var/www/html/modules/node.module on line 1637. 1 notice: Undefined index: 2 in /var/www/html/modules/filter.module on line 629. 1 notice: Undefined index: in /var/www/html/includes/common.inc on line 506. 1 notice: Trying to get property of non-object in /var/www/html/modules/node.module on line 1950. again, no offense, and sorry if i'm trying to bring a dead horse back to life. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:22:44 +0000 : killes(a)www.drop.org This is being worked on for the next Drupal version. Please install yourself a test site on cvs and submit individual patches. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:32:57 +0000 : hlslaughter thanks, i'll do this. i take it patches are just posted here? do i just post a diff -N, title it patch and mark the component appropriately? ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:39:08 +0000 : killes(a)www.drop.org Yes, attach here or create new issue. diff -up is what we usually use. Thanks. ------------------------------------------------------------------------ Tue, 09 Aug 2005 20:24:16 +0000 : hlslaughter Attachment: http://drupal.org/files/issues/test.patch (25.21 KB) attached is results of: harry@penny:~/cvs/third_party/drupal/drupal> diff -Naurp . /var/www/html/drupal_cvs/ > ../test.patch ok, would this type of patch be suitable? it includes fixes for all the E_NOTICE messages generated when accessing the homepage of a new, unmodified drupal installation straight from CVS (as of Aug 9 13:14) ------------------------------------------------------------------------ Tue, 09 Aug 2005 21:01:37 +0000 : killes(a)www.drop.org You are almost there. How to best prepare patches is to run Drupal directly from a cvs checkout. then you can fix the checkout and then run cvs diff -up This way the unrelated files in the patch won't be included. ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:29:12 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/node.user.e_notice.path.txt (1.79 KB) I have a small patch for node.module and user.module that fixes 3 notices in the current HEAD. Please especially have a look at node_access_view_all_nodes, I simply do not get the current use of $node->nid (do I need some more sleep or is this really bogus?) ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:36:22 +0000 : Thomas Ilsche About the newlines at eof: Sorry didn't notcied this as phped removed them manually, is there any reason for them, I got them in many drupal files? Maybe worth a fix aswell? ------------------------------------------------------------------------ Sun, 14 Aug 2005 15:12:00 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/common.inc.enotice.patch.txt (668 bytes) Another patch for common inc. ------------------------------------------------------------------------ Mon, 15 Aug 2005 17:30:34 +0000 : hlslaughter just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). also, sorry i haven't submitted patches, i guess i'm not confident i understand the correct method to submit. is it as follows? 1) cd $drupal_cvs/somedir/ (optionally: cvs update .) 2) edit some.file 3) cvs diff -up > some.file.patch 4) attach some.file.patch to appropriate thread at drupal.org 5) assumming patch is good and correct, pray that someone notices the patch and applies it within CVS ? ------------------------------------------------------------------------ Mon, 15 Aug 2005 18:25:15 +0000 : Thomas Ilsche > just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). I wanted to keep it semantically equivalent to if($var). isset is not. This might be especially critical against boolean values. The very problem is the fact that many properties of objects / indexes of arrays are not initialized. !empty is simply on the safe side. I think with the patches it is important to do them from the main drupal dir and not subdirs (but I'm not sure either, a faq/tut/introduction would be cool) ------------------------------------------------------------------------ Wed, 17 Aug 2005 20:19:36 +0000 : hlslaughter i noticed that some folks are silencing errors instead of doing long-hand 1) not ideal makes "E_ALL" impractical: $foo = $baz["val"]; // generates notice if "val" is not set 2) cleaner: if ( isset($baz["val"]) ) { $foo = $baz["val"]; } // or my preference: $foo = isset($baz["val"]) ? $baz["val"] : NULL; 3) alt way: $foo = @$baz["val"]; methods 1 and 2 can become very ugly and hard to read if you're dealing with long/nested arrays. i like method 3 as it's easy to read and the silencer/@ indicates that the author is aware this syntax may generate a warning notice, but it's ok. what do you guys think?

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Jeremy 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal -Assigned to: Anonymous +Assigned to: Jeremy(a)kerneltrap.org Reported by: Jeremy(a)kerneltrap.org Updated by: Jeremy(a)kerneltrap.org Status: patch (code needs review) Attachment: http://drupal.org/files/issues/comment.module_15.patch (1.18 KB) The attached patch: 1) gets rid of the _cron() hook 2) no longer requires that comments be previewed Prior to this patch, comment spammers were able to send data to a Drupal server acting as though they'd filled out a comment form and pressed submit. As they didn't actually use the form, they could submit spam comments at an obscene rate. With this patch, comment spammers will have to actually load the form, enter text, and press submit. Yes, that can still be automated, but it takes much more work and slows them down, as they have to wait for the entry form to load each time. Unfortunately a spammer could manually submit one comment, then re-use that same session info over and over to attach repeated spam comments to the same node. Such an attempt would be detected and blocked by the spam module if enabled, but again such a session re-use attack could be done without loading the form each time. Fortunately there is much less gain for a spammer to submit 100 spam comments on the same page, versus submitting 100 spam comments each on a different page as they do now. Ideas to improve upon this concept include: - re-key every day or week, changing the private key regularly to be sure it couldn't ever be permanently cracked - add a key table, and generate a unique key for every comment form. essentially, upon comment form creation generate a random key which is stored both in a database table and in the session. when a comment is submited, look for the key from the session in the database table, if a match is found delete it from the database table and post the comment. this would prevent session re-use, but adds overhead. i don't know if it's worth it, perhaps as an external module if the hooks were available. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security. ------------------------------------------------------------------------ Wed, 17 Aug 2005 19:58:02 +0000 : breyten Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it? ------------------------------------------------------------------------ Thu, 18 Aug 2005 03:20:02 +0000 : Jeremy(a)kerneltrap.org > Well, this patch is definitely better than what we have, and would save some spam for sure. It is continuing to work very well on my site, which seems to be under nearly perpetual spam attacks from multiple sources. > But maybe keeping track, at the session level, of generated hashes for a user, and then > removing them when the comment is sent, could do the work. The catch is: the key has to be something unique to the server, not guessable or learnable from the outside Simply storing the hash data in the session alone is not enough, as then the spammer could create any random data and store it in the session. That said, the hash could be generated off something other than the text of the comment as it is now, so that a preview is not required. I'll look at doing something like that and submit another patch. > This way we can forget about previewing comments or not, and also the "permission" to > post the comment would expire when the session expires. Any randomly generated value > could do for this, no need for complex hashes, but having nid and pid in the hash would > add some extra security. nid and pid alone are worthless, as they are easy to learn. The pid can always be 0 (spam is rarely attached to a pre-existing comment). The nid is obtained in the path of where the spam is being posted. The solution is a "private-key", which is what my patch adds. Then sure, hash the private key plus the nid and the pid, and you've got enough protection to prevent most spammers. To make it even more secure, automatic rey-keying could be easily accomplished.

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by Jeremy 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Jeremy(a)kerneltrap.org Updated by: Jeremy(a)kerneltrap.org Status: patch (code needs review) > Well, this patch is definitely better than what we have, and would save some spam for sure. It is continuing to work very well on my site, which seems to be under nearly perpetual spam attacks from multiple sources. > But maybe keeping track, at the session level, of generated hashes for a user, and then > removing them when the comment is sent, could do the work. The catch is: the key has to be something unique to the server, not guessable or learnable from the outside Simply storing the hash data in the session alone is not enough, as then the spammer could create any random data and store it in the session. That said, the hash could be generated off something other than the text of the comment as it is now, so that a preview is not required. I'll look at doing something like that and submit another patch. > This way we can forget about previewing comments or not, and also the "permission" to > post the comment would expire when the session expires. Any randomly generated value > could do for this, no need for complex hashes, but having nid and pid in the hash would > add some extra security. nid and pid alone are worthless, as they are easy to learn. The pid can always be 0 (spam is rarely attached to a pre-existing comment). The nid is obtained in the path of where the spam is being posted. The solution is a "private-key", which is what my patch adds. Then sure, hash the private key plus the nid and the pid, and you've got enough protection to prevent most spammers. To make it even more secure, automatic rey-keying could be easily accomplished. Jeremy(a)kerneltrap.org Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security. ------------------------------------------------------------------------ Wed, 17 Aug 2005 19:58:02 +0000 : breyten Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it?

1 0

[drupal-devel] [task] Similar page returning in update.php
by drumm 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/29006 Post a follow up: http://drupal.org/project/comments/add/29006 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: drumm Reported by: drumm Updated by: drumm Status: patch (code needs review) Attachment: http://drupal.org/files/issues/update.php_0.diff (6.87 KB) This patch moves all of the "which page to show" logic to the bottom of update.php. This code then does the single call to theme('maintenance_page'), similar to how theme('page') is called for the rest of Drupal. This will make future patches shorter and more separatable by page. It will also provide a clear place to put in a progress page. drumm

2 1

[drupal-devel] [bug] fix parse error in drupal. js (is breaking safari)
by moshe weitzman 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/29095 Post a follow up: http://drupal.org/project/comments/add/29095 Project: Drupal Version: cvs Component: base system Category: bug reports Priority: normal Assigned to: moshe weitzman Reported by: moshe weitzman Updated by: moshe weitzman Status: patch (ready to be committed) Attachment: http://drupal.org/files/issues/js.patch (634 bytes) a missing semicolon at end of line is causing a parse error in safari ... once this is fixed, everyone will see that the node/add/x form looks real ugly in safari. it looks like the konqueror screenshots that Ber posted. When you expand/collapse a fieldset, the admin sections jump all over the place. moshe weitzman

1 0

[drupal-devel] [task] Add CVS Id tags to all source files
by Uwe Hermann 18 Aug '05

18 Aug '05

Issue status update for http://drupal.org/node/28482 Post a follow up: http://drupal.org/project/comments/add/28482 Project: Drupal Version: cvs Component: base system Category: tasks Priority: normal Assigned to: Anonymous Reported by: Uwe Hermann Updated by: Uwe Hermann Status: patch (code needs review) Attachment: http://drupal.org/files/issues/ids.patch (7.96 KB) Please add CVS $Id$ tags to all source files in Drupal core. This allows admins to know exactly which version of which files they use. In addition I want to use CVS $Id$ tags in my upcoming security.module to check for possible vulnerabilities. That's not possible if some files simply don't have $Id$s. Note: I also (mostly) unified the tags to use the "// ID" form instead of "/* ID */", but that's more of a cosmetic issue. I'm not sure whether *.txt files and the stuff in themes/ need tags(?) Uwe Hermann

2 7

[drupal-devel] Users self-publishing their content
by Todd Grimason 18 Aug '05

18 Aug '05

Hi all- I asked about this on the support list first and have been looking into it, and wanted to see if you all could confirm my conclusions: I'm running a site(s) which is largely a group weblog (also some "stories"). The authors are trusted to not post nonsense or inappropriate material, but not trusted enough (mostly technically) to have full "admin nodes" privs where they can access each others' work. What they (undestandably IMO) want is the ability to save drafts, i.e. works in progress they don't yet want to publish. With the unmodified core Drupal codebase, I can have the default workflow be "publish" or not. And that works. They get once chance to write it, polish over and over, and hit "submit" -- it's published. Or I set the default to "not published", and they submit it, and then have no way to publish it and I (or another admin) has to publish it. So clearly (right?) I need to find another way to do this, it's not "out-of-the-box-able". A long while ago I tried using workflow.module to accomplish this, and had some issues I can't recall. So I hacked the hell out of a bunch of files, made a mess, and got what I wanted. But not in a resuable way. So this time I've tried to create a clean add-on module, using nodeapi hooks, and it *almost* works cleanly. I can easily get the 'Published' checkbox to appear correctly when the author has the appropriate permissions (in my lil' module), *but* node.module#validate resets the node->status flag on every form interaction. So at this point I stuck in: // "publish my content" is a priv defined in my module if (!user_access("publish my content")) { $node->status = in_array('status', $node_options); } around line 1238 or so in node.module, 4.6.2. Finally, my question: is there any way to do this cleanly -- without modifying core Drupal code? (as currently publishing appears to be an all-or-nothing situation). thanks in advance! === PS: my understanding is workflow.module is in flux right now, with issues with revisions. So I'd guess it won't be ready until post-4.7, but of course I need this sooner - like a week or two. -- ________________________________ toddgrimason*todd[ at ]slack.net

2 2

[drupal-devel] [bug] drupal base and E_NOTICE
by hlslaughter 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/28540 Post a follow up: http://drupal.org/project/comments/add/28540 Project: Drupal Version: 4.6.0 Component: base system Category: bug reports Priority: minor Assigned to: Anonymous Reported by: hlslaughter Updated by: hlslaughter Status: patch (code needs review) i noticed that some folks are silencing errors instead of doing long-hand 1) not ideal makes "E_ALL" impractical: $foo = $baz["val"]; // generates notice if "val" is not set 2) cleaner: if ( isset($baz["val"]) ) { $foo = $baz["val"]; } // or my preference: $foo = isset($baz["val"]) ? $baz["val"] : NULL; 3) alt way: $foo = @$baz["val"]; methods 1 and 2 can become very ugly and hard to read if you're dealing with long/nested arrays. i like method 3 as it's easy to read and the silencer/@ indicates that the author is aware this syntax may generate a warning notice, but it's ok. what do you guys think? hlslaughter Previous comments: ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:19:33 +0000 : hlslaughter ok, instead of whining in the forums, i'll submit a ticket. i'm sure i'm not the first person to bring this up, and i apologize in advance if this issue is already decided/closed. but my issue is this: would it be possible to review the core modules and clean up code to the point that it would be feasible to run drupal with notices turned on (at least in a dev environment)? i think it would be pretty trivial to clean most/all of these errors from the code base. the cost of this effort would be relatively teeny, and the benefits would be great. admins would no longer panic if they happen to see these messages; with an option added to the settings screen, developers could turn on notices and would write cleaner code, which would probably result in fewer bugs in 3rd party modules; etc... you know the routine 8*) after turning on E_NOTICE in common.inc, i see the following notice messages when accessing the homepage of my site: occurences - message ----V--------------V-------- 12 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 114. 10 notice: Undefined variable: teaser in /var/www/html/modules/opt/attachment.module on line 37. 6 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 120. 6 notice: Undefined index: field in /var/www/html/includes/tablesort.inc on line 176. 4 notice: Undefined variable: rows in /var/www/html/modules/opt/attachment.module on line 352. 4 notice: Undefined variable: count in /var/www/html/modules/opt/attachment.module on line 123. 4 notice: Undefined variable: attachments in /var/www/html/modules/opt/attachment.module on line 279. 1 notice: Undefined variable: node in /var/www/html/modules/node.module on line 1950. 1 notice: Undefined offset: 0 in /var/www/html/includes/pager.inc on line 59. 1 notice: Undefined index: op in /var/www/html/modules/node.module on line 1636. 1 notice: Undefined index: node.taxonomy in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: node.picture in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.title in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.site_slogan in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.mission in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: header.help in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: from in /var/www/html/includes/pager.inc on line 53. 1 notice: Undefined index: footer.message in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: footer.blocks in /var/www/html/themes/engines/xtemplate/xtemplate.inc on line 164. 1 notice: Undefined index: edit in /var/www/html/modules/node.module on line 1637. 1 notice: Undefined index: 2 in /var/www/html/modules/filter.module on line 629. 1 notice: Undefined index: in /var/www/html/includes/common.inc on line 506. 1 notice: Trying to get property of non-object in /var/www/html/modules/node.module on line 1950. again, no offense, and sorry if i'm trying to bring a dead horse back to life. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:22:44 +0000 : killes(a)www.drop.org This is being worked on for the next Drupal version. Please install yourself a test site on cvs and submit individual patches. ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:32:57 +0000 : hlslaughter thanks, i'll do this. i take it patches are just posted here? do i just post a diff -N, title it patch and mark the component appropriately? ------------------------------------------------------------------------ Tue, 09 Aug 2005 19:39:08 +0000 : killes(a)www.drop.org Yes, attach here or create new issue. diff -up is what we usually use. Thanks. ------------------------------------------------------------------------ Tue, 09 Aug 2005 20:24:16 +0000 : hlslaughter Attachment: http://drupal.org/files/issues/test.patch (25.21 KB) attached is results of: harry@penny:~/cvs/third_party/drupal/drupal> diff -Naurp . /var/www/html/drupal_cvs/ > ../test.patch ok, would this type of patch be suitable? it includes fixes for all the E_NOTICE messages generated when accessing the homepage of a new, unmodified drupal installation straight from CVS (as of Aug 9 13:14) ------------------------------------------------------------------------ Tue, 09 Aug 2005 21:01:37 +0000 : killes(a)www.drop.org You are almost there. How to best prepare patches is to run Drupal directly from a cvs checkout. then you can fix the checkout and then run cvs diff -up This way the unrelated files in the patch won't be included. ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:29:12 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/node.user.e_notice.path.txt (1.79 KB) I have a small patch for node.module and user.module that fixes 3 notices in the current HEAD. Please especially have a look at node_access_view_all_nodes, I simply do not get the current use of $node->nid (do I need some more sleep or is this really bogus?) ------------------------------------------------------------------------ Thu, 11 Aug 2005 16:36:22 +0000 : Thomas Ilsche About the newlines at eof: Sorry didn't notcied this as phped removed them manually, is there any reason for them, I got them in many drupal files? Maybe worth a fix aswell? ------------------------------------------------------------------------ Sun, 14 Aug 2005 15:12:00 +0000 : Thomas Ilsche Attachment: http://drupal.org/files/issues/common.inc.enotice.patch.txt (668 bytes) Another patch for common inc. ------------------------------------------------------------------------ Mon, 15 Aug 2005 17:30:34 +0000 : hlslaughter just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). also, sorry i haven't submitted patches, i guess i'm not confident i understand the correct method to submit. is it as follows? 1) cd $drupal_cvs/somedir/ (optionally: cvs update .) 2) edit some.file 3) cvs diff -up > some.file.patch 4) attach some.file.patch to appropriate thread at drupal.org 5) assumming patch is good and correct, pray that someone notices the patch and applies it within CVS ? ------------------------------------------------------------------------ Mon, 15 Aug 2005 18:25:15 +0000 : Thomas Ilsche > just curious, any special reason you're using "!empty()" instead of "isset()"? i'd think that an empty string is technically a valid value (though the end result is the same codewise). I wanted to keep it semantically equivalent to if($var). isset is not. This might be especially critical against boolean values. The very problem is the fact that many properties of objects / indexes of arrays are not initialized. !empty is simply on the safe side. I think with the patches it is important to do them from the main drupal dir and not subdirs (but I'm not sure either, a faq/tut/introduction would be cool)

1 0

[drupal-devel] [bug] comment preview "Required" is easily bypassed
by breyten 17 Aug '05

17 Aug '05

Issue status update for http://drupal.org/node/28420 Post a follow up: http://drupal.org/project/comments/add/28420 Project: Drupal Version: cvs Component: comment.module Category: bug reports Priority: normal Assigned to: Anonymous Reported by: Jeremy(a)kerneltrap.org Updated by: breyten Status: patch (code needs review) Jeremy, a big +1 on the idea, but why not generate the private key when it is actually needed (Ie, when displaying the comment form), instead of wasting a _cron() hook on it? breyten Previous comments: ------------------------------------------------------------------------ Mon, 08 Aug 2005 01:55:34 +0000 : Jeremy(a)kerneltrap.org Setting "Preview comment" to "Required" does not strictly require that the comment be previewed first. This is being abused by spammers to quickly and efficiently post spam comments. I discovered this after I added a new feature to my new spam module [1] to auto-blacklist spammer IP addresses, allowing me to block comment spammers when they preview a comment and thus preventing them from ever inserting their spam into my database. I configured my comment module to "require" comment previews, and yet found that the comments were slipping past my filter. I finally realized what the spammer is doing is setting $_POST['op'] to 'Post comment', effectively bypassing the preview phase. I'm currently looking for a clean solution to this. At the moment the only idea I have is to generate a token at the preview phase, and validate the token at the post phase. Unfortunately the token would have to be stored in the databse between the preview and the post, which adds overhead. Alternatively, I've considered using a time-based hash which would constantly update depending on the time of day. This could easily be validated without storing anything in the database. If too long has gone between the preview and the post, an additional preview step would be required... The down side here is that the time-based hash would be publically available, and thus the spammer could easily duplicate it in their script. A private key could solve for that, but increases the complexity as it adds a configuration step. I have the feeling I'm missing a simpler, cleaner solution. Suggestions? [1] http://kerneltrap.org/jeremy/drupal/spam/ ------------------------------------------------------------------------ Mon, 08 Aug 2005 02:26:21 +0000 : moshe weitzman even if you get this fixed, won't these bots just add a preview step? this 'preview required' feature is designed to maintain high quality submissions by forcing users to proof read. it isn;t designed for security. i think you want to hook into comment_validate(). just add a hook here - there is already a hook_comment() waiting for you to add an operation. ------------------------------------------------------------------------ Mon, 08 Aug 2005 08:49:30 +0000 : Eaton I posted a patch a few days ago (http://drupal.org/node/28255) that adds validation and form construction hooks for comments. It's similar to the one that the captcha module uses, though it adds comment form_pre and form_post hooks instead of a single comment form hook. ------------------------------------------------------------------------ Mon, 08 Aug 2005 13:30:34 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_11.patch (2.5 KB) > even if you get this fixed, won't these bots just add a preview step? Eventually, yes, but it drastically changes their ability to fling spam at a site. As is, they simply have a script that shoots data out at high speed without having to wait for messages to return from the server. It is the server that is doing all the work, thus making it simple for a spammer to DoS a site. If "preview required" really meant "preview required", they would be forced to first automate clicking "preview", and then wait for a response before clicking "submit". This requires more resources on their side, and allows us to add delays after clicking "preview" (if we detect that they are a spammer) further using their resources. > this 'preview required' feature is designed to maintain high quality > submissions by forcing users to proof read. it isn;t designed for security. Regardless of the intention, I was misled to believe that configuring my site to require previews would require that all comments were first previewed. As a site administrator, I would prefer to know that "required" really means "required". > i think you want to hook into comment_validate(). just add a hook here - > there is already a hook_comment() waiting for you to add an operation. Yes and no. Ultimately yes that will work and will allow my spam module to prevent the spam from ever being posted. But it still leaves the greatest burden on the web server, instead of on the spammer. The spammer can still use a very simple script that only pushes data, and thus can generate spam at an unbelievable rapid rate. Here is an example patch to enforce "preview required". It's one idea, I'm sure there are better ones. ------------------------------------------------------------------------ Mon, 08 Aug 2005 14:01:38 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_12.patch (1.4 KB) Here's a second version of the patch that doesn't require any manual configuration. ------------------------------------------------------------------------ Mon, 08 Aug 2005 16:27:27 +0000 : Jose A Reyero I like this idea, and the patch looks good Still, I think it misses something, like some timestamp related hash, because once you get the hash code you can post multiple comments with that. Another problem I can think of is, what happens when a cron run happens between the preview and the post?? I'm afraid comments would get lost For this second problem, I think a key generated only once after module activation could do. About the first one....mmm... I'll sit down for a while and think..... ------------------------------------------------------------------------ Tue, 09 Aug 2005 12:27:20 +0000 : Jeremy(a)kerneltrap.org Attachment: http://drupal.org/files/issues/comment.module_13.patch (1.33 KB) > I think it misses something, like some timestamp related hash, because > once you get the hash code you can post multiple comments with that. Using a timestamp will mean that the comment form "expires". That is, if you wait too long to preview your comment, it will generate an error when you try to post. Yes, technically a spammer could post one real comment, then based on what was in the session from that they could post the same identical comment over and over, so long as it was attached to the same node. But this is not what they do, they try and spread their spam throughout your webpage. Furthermore, the spam module is perfectly capable of detecting and preventing this. > Another problem I can think of is, what happens when a cron run happens > between the preview and the post?? I'm afraid comments would get lost The key is only generated once, that's what the first test is about. In any case, in the unlikely event that the key were to change between preview and post they would simply have to post a second time. My earlier patch wasn't quite right, I was testing the token in the wrong place. This patch fixes that. BTW: This is beneficial for maintaining high quality submissions too, as prior to this change someone could: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and the comment (mistake and all) would go into the database unpreviewed After this change: 1) enter a comment 2) press preview 3) completely change their comment (introducing a mistake) 4) press submit and they get an error because they didn't preview their changes - forcing them to preview once after any change ------------------------------------------------------------------------ Wed, 10 Aug 2005 03:50:33 +0000 : Jeremy(a)kerneltrap.org FWIW: I've been getting slammed by spam attacks this whole week. Installing this patch has made a huge difference. Well over 100 spam attempts per minute (sometimes two and three times that) and I hardly notice the spammer, whereas before it was choking my database. (Granted, the spammer has not yet upgraded his script to first preview, then submit. But even if he did it wouldn't help him as testing has verified that the new spam module would prevent the comments from ever getting to the database.) Additionally, user and anonymous (nonspam) comments continue to show up at a normal rate. ------------------------------------------------------------------------ Tue, 16 Aug 2005 14:08:04 +0000 : Jeremy(a)kerneltrap.org I would love to see _any_ discussion on this. Drupal is currently too easy to spam, with little effort on the spammer's side, and lots of resources wasted on the Drupal side. A patch like this will greatly increase the spammer's burden, and make it possible to effectively block even the most aggressive spammer attacks. ------------------------------------------------------------------------ Wed, 17 Aug 2005 16:24:04 +0000 : Jose A Reyero Well, this patch is definitely better than what we have, and would save some spam for sure. But maybe keeping track, at the session level, of generated hashes for a user, and then removing them when the comment is sent, could do the work. This way we can forget about previewing comments or not, and also the "permission" to post the comment would expire when the session expires. Any randomly generated value could do for this, no need for complex hashes, but having nid and pid in the hash would add some extra security.

1 0