- Development - lists.drupal.org

Porting - Quick security reminders
by Heine Deelstra 04 Jan '07

04 Jan '07

Dear all, Porting a module is an excellent opportunity to keep an eye out for security problems (evidence: <http://drupal.org/node/103943>). Here's a quick security reminder regarding input (user-supplied data). Code samples are only included to make a point, do not hold them against me. Escape or filter text before display ==================================== If you do not escape or filter text before display, you enable a user to insert arbitrary HTML and scripts into pages. This type of attack is know as cross site scripting aka XSS. For more information, see: <http://drupal.org/node/28984> Escape / filter: - check_plain or theme('placeholder') for plain text - check_markup or filter_xss for markup containing text Drupal 5 brought a (brilliant IMO) change to t(): Depending on the placeholder's sigil, it is passed through theme('placeholder') (%) or check_plain (@) automatically. t('I escape %user_data', '%user_data'); // I escape <em>user_data</em> (safe) t('I escape @user_data', '@user_data'); // I escape user_data (safe) t('I don't escape !user_data', '!user_data'); // XSS vulnerability Sometimes the difficult part is to know whether a function requires escaped text or does the escaping itself: * Automatically escapes: l() - Escapes text and attributes unless you pass TRUE for the $html parameter. In that case only attributes are check_plained. - menu items & breadcrumbs - block descriptions (*not* titles) - theme('username') * Common pitfalls: watchdog - you have to provide a safe $message drupal_set_title - you have to provide a safe $title Form elements - #description and #title of form_elements need to be safe Form elements - #value of #type markup and item need to be safe. Note that the default form element #type is markup! Examples: drupal_set_title($node->title); // XSS vulnerability drupal_set_title(check_plain($node->title)); watchdog(content', t("Deleted !title", array('!title' => $node->title)); // XSS watchdog(content', t("Deleted %title", array('%title' => $node->title)); // or @ $form['unsafe'] = array('#value' => $user->name); //XSS $form['safe'] = array('#value' => check_plain($user->name)); or $form['safe'] = array('#value' => theme('username', $user)); $form['bad'] = array( '#type' => 'textfield'; '#default_value' => check_plain($u_supplied), // bad: escaped twice '#description' => t("Old data: !data", array('!data' => $u_supplied)), // XSS ); $form['good'] = array( '#type' => 'textfield'; '#default_value' => $u_supplied, '#description' => t("Old data: @data", array('@data' => $u_supplied)), ); Database (ab)use ==================================== See also <http://drupal.org/node/101496> Summary: Whatever you do, *never* use user supplied data directly in a query. Use db_query as it was meant to be used: data goes in arguments. And make sure to enclose %s in single quotes: '%s'. BAD, BAD, BAAAD: 1.db_query("SELECT t.s FROM {table} t WHERE t.field = $from_user"); 2.db_query("SELECT t.s FROM {table} t WHERE t.field = %s", $from_user); Good: db_query("SELECT t.s FROM {table} t WHERE t.field = '%s'", $from_user); BAD ($from_user is an array of numbers): db_query("SELECT t.s FROM {table} t WHERE t.field IN (%s)", $from_user); Good: $placeholders = implode(',', array_fill(0, count($from_user), "%d")); db_query("SELECT t.s FROM {table} t WHERE t.field IN ($placeholders)", $from_user); Anything else: bad. Why is this bad? Suppose you check the data that you stuff in to a query (e.g. 'all numeric'). Or, you know a user name is validated to contain only certain 'safe' characters. The problem is that you open the possibility that a code path develops where unfiltered content ends up at your query. In that case you're toast. This code path can arise because you 1) accidentally remove validation in future development, 2) a user adds a module that is more lenient in the data it accepts or 3) you add a bunch of code that skips validation (recent xTracker problem). Writing secure code ==================================== If you want to read more about security, I suggest that you start at "Input the root of all evil" <http://drupal.org/node/101495> in the "Writing secure code" <http://drupal.org/node/62304> pages. I hope to expand and improve those pages in the future, but some of them are already useful. You found a vulnerability? ==================================== What to do when you find a vulnerability in your module? Simple; write us at security(a)drupal.org. Thank you for listening, Heine Deelstra

5 7

Simple XML Import
by Dave Fletcher 04 Jan '07

04 Jan '07

Hi all. I created a quick XML import method and I'm wondering if others are interested in it. This thing defines a simple XML format with tags: drupal (doc element), vocab, term, user, node, comment. It imports each of these things by calling the relevant xxx_save() function. Each of those tags can define a "fields" child element. The children of <fields> are turned into object, array, or primitive values and passed to the save func. Node elements located inside term elements are categorized by that term. The system holds a legacy ID which is mapped to a Drupal ID. It is used to ensure a piece of content is not imported multiple times. Therefore, an XML document can be updated and imported again without causing dupes. The system can also use this ID to reference existing content, for example in a uid field or the taxonomy array. Since it's just calling node_save to do all the work, this system can import custom node types simply by providing the right 'type' field in a node element and providing the fields that the custom node module expects. I have imported 5 different content types using this method: a custom review type, polls, blog (converted the mt2drupal script to this XML format), a custom newsletter type, and galleries. I of course know about Jaza's work on import/export API but it seemed like an overkill solution to me and I didn't want to really learn an API. I just wanted a quick XML format to do some dirty work which I tried to do by hand on the first rev. The XML format mainly allows me to inspect the data before I run the import. So my question for you, fellow Drupallers, is does this sound useful to you? Would it be worth my time turning this thing into a real module? Currently it's a command-line hack that lives in Drupal root directory and bootstraps. It would be neat if there was a module page, an upload form, and perhaps some support functions for creating the XML format from code. Cheers, --fletch

3 3

I unsubscribe
by Karoly Negyesi 04 Jan '07

04 Jan '07

Hi, Apparently it is more important to write mails that make people feel welcome here than to develop and I am actually advised not to read the list or at least parts of it. So, I unsubscribe now and stuck to places where actual development happens: the issue queue and the channel. Yesterday a totally new guy proposed an interesting idea on #drupal and in private and today the patch is submitted. I like this. Still I will post questions/proposals to the list occassionally and read the archives but I am off the list. All the best, Karoly Negyesi

2 2

Please test http://drupal.org/node/106370
by Gerhard Killesreiter 03 Jan '07

03 Jan '07

Hi there, yesterday a bug was fixed in Drupal 5RC, but I'd like to backport the fix to 4.7. I'd also like to release 4.7.5 soon, so some speedy help would be appreciated. It would be useful if people could test the patch on either 4.7 or 5RC sites (for 5-HEAD it is already in cvs) and report back on the issue tracker. Testing is only relevant if you use a node access module such as OG, tac_lite or one of the others. Please report your results on the issue tracker stating the node access module you use. Cheers, Gerhard

1 0

Re: [development] Community, mailing list and IRC (was: I unsubscribe)
by Khalid B 03 Jan '07

03 Jan '07

Karoly Please allow me to respond. On 1/3/07, Karoly Negyesi <karoly(a)negyesi.net> wrote: > > Apparently it is more important to write mails that make people feel > welcome here than to develop and I am actually advised not to read the > list or at least parts of it. It is important (not more important, just important) to welcome and encourage new comers. Correcting their mistakes in a non-confrontational, non-frictional way would most likely cause us to gain one more developer, as well as a better sense of community as a whole, not to mention the community reputation. So, I unsubscribe now and stuck to places where actual development > happens: the issue queue and the channel. Yesterday a totally new guy > proposed an interesting idea on #drupal and in private and today the patch > is submitted. I like this. Karoly, IRC is a valuable collaboration tool, but not for everybody. Others, including myself, find the "everyone talking to everyone" distracting, and just aggravates the ADD ( http://en.wikipedia.org/wiki/Attention-deficit_hyperactivity_disorder) factor. Moreover, IRC is transient. One cannot go back and check what has been discussed, or what drawbacks a certain approach has, ..etc.. Dries has repeatedly stated that IRC is not an official communication channel because of the above reasons. We've also seen some "surprise" decisions for the rest of the community when a small group discusses/decides on IRC and then things are implemented without proper notification to the rest of the community. So, by all means, use it for collaboration and whatever you find useful, but don't make it the sole conduit for everyone else.

1 0

Drupal 4.7 - Question regarding Comments module.
by Ignacio G. Dupont 03 Jan '07

03 Jan '07

Hi, I have had a problem with the comments module (administer->comments) and I have a couple of questions on it. I'm currently using Drupal 4.7.4 with PHP 4.4.3 and MySQL 4.1.21 When entering the configure tab I was getting this warnings. warning: array_keys() [function.array-keys]: The first argument should be an array in /home/server/public_html/modules/user.module on line 352.warning: implode() [function.implode]: Bad arguments. in /home/server/public_html/modules/user.module on line 352.user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 query: SELECT DISTINCT(p.perm) FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN () in /home/server/public_html/includes/database.mysql.inc on line 121.After debugging it a bit I found that when looking for the 'post comments' permissions it was hard sending a user with uid = 0. I have recently had to recover my database and row with uid = 0 on users table was missing. As my Administrator user has uid = 1 I tried to replace the hardcoded value to 1: --- a/comment.module 2006-10-18 15:14:42.000000000 -0500 +++ b/comment.module 2007-01-03 03:17:09.000000000 -0600 @@ -402,7 +402,7 @@ COMMENT_ANONYMOUS_MUST_CONTACT => t('Anonymous posters must leave their contact information')), '#description' => t('This option is enabled when anonymous users have permission to post comments on the <a href="%url">permissions page</a>.', array('%url' => url('admin/access'))), ); - if (!user_access('post comments', user_load(array('uid' => 0)))) { + if (!user_access('post comments', user_load(array('uid' => 1)))) { $form['posting_settings']['comment_anonymous']['#attributes'] = array('disabled' => 'disabled'); } It seemed to work ok. I have performed a grep over all module sources and it seems that is the only place where the seccond parameter is hardcoded when calling user_access function: user@server [~/public_html/modules]$ grep "user_access([^,)]*," * blog.module: if ($type == 'view' && user_access('edit own blog', $user)) { blogapi.module: if (user_access('edit own blog', $user)) { comment.module: if (!user_access('post comments', user_load(array('uid' => 0)))) { user.module:function user_access($string, $account = NULL) { I then decided to undo the change in the comment.module and go on with an insert into the users table with uid = 0 and the rest of the columns with their default values and it seems to work ok. What I don't understand is why do you pass the second argument instead of having the default value (NULL) to be assigned to the $account parameter in the user_access functiont of the user.module: function user_access($string, $account = NULL) { global $user; static $perm = array(); ... Regards. Ignacio. __________________________________________________ Correo Yahoo! Espacio para todos tus mensajes, antivirus y antispam ¡gratis! Regístrate ya - http://correo.yahoo.es

1 0

Re: [development] OT: Future of Transactional Engine with MySQL
by Steven Peck 03 Jan '07

03 Jan '07

> -----Original Message----- > From: development-bounces(a)drupal.org [mailto:development- > bounces(a)drupal.org] On Behalf Of Dries Buytaert > > > I've talked to some MySQL employees and it seems that Falcon is going > to be the new InnoDB -- so to speak. ;-) It's developed internally > at MySQL as a replacement for InnoDB. > > In general, it is expected that more and more storage engines will > emerge. There are even plans (jokes?) to create a mail storage > engine (with header parsing support) and/or an image storage engine > (with EXIF support). Imagine being able to query images based on the > embedded EXIF data -- sure sounds like fun. Actually there would need to be something like that developed to compete with Exchange which uses a highly optimized jet engine to accomplish that but that's a different list discussion ;).

1 0

Missing CVS branches in contrib -- peeve
by Chris Johnson 03 Jan '07

03 Jan '07

This is a pet peeve. Be forewarned. :-) Today, for the Nth time, I again wasted time looking for a module in the 4.7 branch of the contrib repository that was not there. Why? Because it was in the HEAD branch, even though it only works with 4.7 -- and not 5.0 or later. So this brings me to my pet peeve of the moment: module maintainers who do not branch their modules when appropriate. How can this be made easier for people, and more reliable for folks who want to download or checkout contrib code?

6 5

Re: [development] Missing CVS branches in contrib -- peeve
by Metzler, David 03 Jan '07

03 Jan '07

It seems to me that the docs conflict just a bit. I like the concept of using head for new feature module releases, creating releases when they're stable, and doing a branch when you need to cause the API's underneath change. Every time you branch, it seems to imply that you're going to maintain that branch for bug fixes, and I don't do that (don't have time). I will probably maintain head for the current stable version, and when I decide it's time to port, I'll create a branch to maintain bugfixes only in the prior stable release version, and continue new feature development in HEAD. Regardless as to how many branches I create (cause the docs tell me to) I'm trying to see if I'm missing something here. The only reason I bring this up, is that I've created a 5.0 branch on my code that I will "never alter any code in" because the doc's suggested I'd be irresponsible if I didn't. Shouldn't I have waited till 5.0 released before creating this branch? Is there a reason I shouldn't develop 5.x-0.1 in HEAD and then tag new releases from that?. (I've already created a DRUPAL-4-7 branch to maintain my 4.7 compatible code in and created new releases from it). Am I being "irresponsible"? What am I missing here? Decide for yourself whether you think this is in conflict: On the one hand...... http://drupal.org/handbook/cvs/branches-and-tags/contributions Once a contribution has been ported to a given version of the Drupal core API, the maintainer should create a branch that matches the branch name used by core. This is not technically required, but is definitely recommended for responsible project maintainers. These initial stable branches have the exact same format as Drupal core branches, for example DRUPAL-5 or DRUPAL-4-7. On the other..... http://drupal.org/node/17570#HEAD Eventually, you'll decide it is safe to update the module to the next version of core. At this point you should make a new branch for your previously developed features, before you start porting to the latest version of core. Once that branch was created, you would convert your module in HEAD, and when you consider it stable enough for an official release, you would create a new stable branch that was compatible with the new version of core. Thanks again Derek for the new release system... I am loving it! Dave

2 1

Not releasing for 5: remove HEAD alltogether?
by Bèr Kessels 03 Jan '07

03 Jan '07

Hi there. I maintain two projects that have better options alread in 4.7, I spent that cycle to phase them out and don't plan a release for 5. Should I remove the HEAD version from CVS? (cvs remove, so still accessible, for those interested) Bèr -- Drupal, Ruby on Rails and Joomla! development: webschuur.com | Drupal hosting: www.sympal.nl

3 2