A security vulnerability was called to my attention, and because of it the module was marked as "unsupported". The announcement was made here before I could get a fix...
http://drupal.org/node/1471800

The "Add Site" or "Edit Site" admin form opens up a possible SQL injection with the "table prefix" value. Here's a patch that adds a validate function, and only accepts values that are alphanumeric, underscores, and periods.

(TODO: Look into using db_escape_table() to clean up the queries, although I have to account for periods as well.)

CommentFileSizeAuthor
#6 multisite-search-search-67542-2.patch10.55 KBearthday47
multisite_search-security-67543-1.patch1014 bytesearthday47

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I am following this issue and happy to review patches and, once the code is good and committed, get the project back to full status.

Thanks for working on this.

money2k’s picture

money2k commented

~line: 285

//validate table prefix
if (preg_match('/^[a-zA-Z0-9_\.]*$/', $result['table_prefix'])){
//code goes here that handles datasets, words, counts, etc
}

earthday47’s picture

earthday47
Location New York
commented
Status: Active » Needs review

Cool, that's the solution I wrote in the patch :)

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Needs work

I think db_escape_table in every queyr is an important second part of the solution.

earthday47’s picture

earthday47
Location New York
commented

The current implementation allows table_prefix to have periods as well, for the database name, like dbname1.db_table_prefix_

I guess I should change the implementation, but it's a lot of work :(

earthday47’s picture

earthday47
Location New York
commented
Status: Needs work » Needs review
StatusFileSize
new multisite-search-search-67542-2.patch10.55 KB

Please try the attached patch. (ignores the last patch)
Run update.php after applying.

This patch splits the "db_prefix" and "table_prefix" values into two database columns. Then, wherever possible, both values are run through db_escape_table().

-Wes

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

The sections you fixed look good to me. Thanks for your work.

chales’s picture

chales commented

I installed the module and tested to see that there was an issue with the 'table prefix' field as mentioned. I then applied the patch which validates the submitted field data and removes the issue. This seems to have fixed the issue.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@earthday47 - given chales review you can go ahead and commit the patch and create the release. Once that's done comment here and we will update the project page to make it a supported project again.

earthday47’s picture

earthday47
Location New York
commented
Version: 6.x-2.2 » 6.x-2.3
Status: Needs review » Fixed

\I just committed the patch to 6.x-2.x, and created a 6.x-2.3 release: http://drupal.org/node/1649974
I marked it as a security release.

Thanks for the reviews everyone, looking forward to having this back in business!

-Wes

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Alrighty, the project should be editable by you again and that release is published.

Thanks, Wes.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.