Prevent Malicious Title Generation
stripped your speech - August 8, 2008 - 15:02
| Project: | Facelift Image Replacement Integration |
| Version: | 5.x-1.2 |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Description
I added this into generate.php to prevent people from creating their own URL string to generate an image.
After line 66, I altered it to do:
$text = isset($_GET['text'])?strip_tags($_GET['text']):'unknown';
$text = preg_replace_callback('#\%u[a-f0-9]{4}#i', 'replace_unicode', $text);
$databaseTable = mysql_connect("localhost", $username, $password);
mysql_select_db('tablename', $databaseTable);
$query = "SELECT n.title, n.status FROM node n WHERE n.title = '$text' AND n.status = 1";
$queryTitle = mysql_query($query, $link);
if ( mysql_num_rows($queryTitle) == 1 ) {Then be sure to close your if statement at the bottom of the file. Now any URL that isn't a title in the Drupal node system won't be generated if directly requested.
#1
Edit:
You'll want to place it just before the line begining with $cache = ... etc
I updated the query to look for all titles through node revisions:
$query = "SELECT n.nid, n.title, n.status, nr.nid, nr.title FROM node n INNER JOIN node_revisions nr ON nr.nid = n.nid WHERE nr.title LIKE '$text' AND n.status = 1";
$queryTitle = mysql_query($query, $link);
if ( mysql_num_rows($queryTitle) >= 1 ) {
#2
What happens if you want to alter some other property though? This module (technically) allows you to change any text, not just title text. If you wanted to use this to make your drupal feedback messages look snazzy you could. Or if you wanted to use it on your site name in the site's theme name you could do that also. We need someway of blocking access unless the referring page is on "this" server.
#3
The new version creates a permission and also uses drupal paths to generate it instead of the generate.php file. This will help to alleviate this somewhat if you don't want annon people being able to generate images. I'll still gladly take a patch of the new version if you come up with anything for blocking external requests.
#4
Using the functions drupal_get_token() and drupal_valid_token() might be a cleaner way to prevent malicious uses of the generator. We'd create a token when building the page, then pass it in the URL to the generate callback, which would generate the image only if the token is valid.
#5
You can also block anon people from using it via the permission since it is a drupal path (I believe).
#6
Hmm, sure using the permissions would be safer, but then users could still abuse the system just by being authenticated.
More importantly, if text replacements are not available to anonymous users, I don't think it will be worth setting up FLIR on many systems...
The token solution should be quite easy to implement. I might even provide a patch at some point... unless the issue is closed... ;-)
#7
If you'd like to patch and jump on as a co-maintainer then please let me know. This version's pretty dang powerful so I don't feel that it meets our needs internally for me to keep up with it unless the Facelift project goes up to some even crazier, better version. Let me know.
#8
I'll see if in the end I can use it in my projects. If so, I might be able to contribute more. The difficult thing I see in keeping this module up-to-date are the code changes required in Facelift to make it work with Drupal. At the moment, it's a bit like a fork of the original Facelift. Because of this, I fully understand the problem you are facing! I guess close collaboration with the author would be required to allow Facelift to be integrated with Drupal without having to make those changes. Have you been in contact with the author? Has he shown openness to this kind of improvements? (sorry if I'm venturing too far off-topic...)
#9
ha, no problem at all. Yes I've been in touch with Cory, Facelift's author and have actually gotten a few Drupal-like pieces of functionality into facelift (like the option to scan the fonts folder instead of manually activating in a configuration file). The integration of the Drupal version is actually pretty similar to the facelift project that's on his website. The only differences are 2 line changes in the facelift.js file so that it goes to the ?q= path and I take one of the files he writes and encapsulate it in a function (the generator function) as well as some patches to a config. file to play nice with Drupal's file system. The files in flir/flir are actually the same between D5 and D6 and almost the same as what's included with the facelift project.
The author has also been very helpful in helping me hunt down what's wrong in my code or at least pointing me in the right direction for ports. The upgrade from 1.1 to 1.2 was pretty easy, I just messed up something with some global vars that needed to be shared between the flir files and module.
#10
Good to hear that! I hope to be able to give 1.2 a try soon. Thanks for all the info!