Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
16 Oct 2012 at 09:58 UTC
Updated:
27 Jan 2013 at 10:42 UTC
The Date Range module implements a custom field which allows you to enter dates and periods as a text string with an arbitrary precision, e.g.:
1991 - a period, consisted of all days of year 1991;
2012.02 - all days of February 2012;
1917.10.25 - October 25, 1917 exactly;
1941-1945 - a period from the beginning of 1941 till the end of 1945;
1234.05.06-7890 - a period from May 06, 1234 till the end of year 7890.
A general pattern for field input is: YYYY[.MM[.DD]][-YYYY[.MM[.DD]]].
The module also implements a custom filter for Views which allows you to select nodes which are before, after, inclide, exclude or exactly equal to the specified period.
Module has versions for Drupal 6 and 7.
Project Page:
https://drupal.org/sandbox/RedRat/1750898
Git:
git clone --recursive --branch 7.x-1.x http://git.drupal.org:sandbox/RedRat/1750898.git date_range
git clone --recursive --branch 6.x-1.x http://git.drupal.org:sandbox/RedRat/1750898.git date_range
Comments
Comment #1
cubeinspire commentedHi,
There are security issues from coder:
Module file
Line 106, FAPI elements '#title' and '#description' only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
'#title' => t($instance['label']),Line 133, form_set_error() and form_error() only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
form_error($element, $var);date_range_handler_filter.inc file
Line 158, form_set_error() and form_error() only accept filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized.
form_set_error($identifier, $var);There are alos code standard problems :
http://ventral.org/pareview/httpgitdrupalorgsandboxredrat1750898git
Comment #2
RedRat commentedLine 106, FAPI elements '#title' and '#description' only accept filtered text
Now I'm rewriting this function and there will be no more #title here.
Line 133, form_set_error() and form_error() only accept filtered text
$var may contain only some predefined hardcoded strings so I think it's pretty safe to use it here without check_plain(), filter_xss() or similar.
Line 158, form_set_error() and form_error() only accept filtered text
The same situation as above.
There are also code standard problems
I am working on it right now, but it seems almost all of this "errors" a false positive, because code for this object was borrowed from a standard Fields module.
Comment #3
RedRat commentedModule code is polished and ready for review.
Comment #4
anton-staroverov commentedHi RedRat,
Good idea but need to be polished.
1) There are still many errors at http://ventral.org/pareview/httpgitdrupalorgsandboxredrat1750898git
2) Mmm. I think the main goal for success of this project is to integrate it with powerful Date module. Do you plan to do that?
3) I think you have to remove all the comments like "// dpm($entity, 'formatter_view $entity');" from the code. Because they needed only for debugging, not for production. It's better to use these statements in separate or a local branch I think.
4) It would be great if you provide more real comments though the code. For now there are no comments except dpm() calls.
Comment #5
RedRat commented1) All mentioned errors in fact are not an errors:
+109: [critical] Potential problem: FAPI elements '#title' and '#description' only accept filtered text
t() call uses check_plain() inside, so a string in this code line is sanitized for sure.
+177: [critical] Potential problem: form_set_error() and form_error() only accept filtered text
As in previous case, returned value in this code is already sanitized by a t() call.
+70: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
+71: [minor] in most cases, replace the string function with the drupal_ equivalent string functions
Proposed drupal_ equivalent string functions doesn't has any advantages here because (cite Drupal documentation): "Note that for cutting off a string at a known character/substring location, the usage of PHP's normal strpos/substr is safe and much faster."
It seems that any other code styling errors of Views handler are false too, because code of handler was borrowed from a standard Views string handler.
2) I had such plans from the very beginning, but there is no any documentation about inner structures of (very large, powerful and tangled) Date module, and all my questions on forum and support requests in issue queue remains without any answers. So I had no choice but write it all myself. At least, there is a pretty good documentation for Fiels API and Form API, and not so good (but better than nothing) documentation for Views.
But I can rewrite this module just as a custom widget/formatter for Date module if someone clue me up.
3) Yes, you are right. I'll remove these comments just before public release.
4) Well, it's a big deal for me because my English is a really awkward. But I will try! ;-)
Comment #6
alex.sorokin.v commentedHi!
Here is the report from manual review done of your code:
1. 'date_range.inc'-file line 31 and 32. You can add 's' attribute in regexp. It's help to do global search, if input data will be multi-string.
2. Please remove all commented line with debug info, like in 'date_range_handler_filter.inc'-file lines 250-258
3. Please remove line 109 from 'date_range_handler_filter.inc' or change to: '#title' => empty(filter_xss($form_state['exposed'])) ? t('Value') : '',
Also please fix all issues from http://ventral.org/pareview/httpgitdrupalorgsandboxredrat1750898git
-regards
Comment #7
klausiClosing due to lack of activity. Feel free to reopen if you are still working on this application.
If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)