If there's a character that requires escaping in file_directory_path() or $dirname, the SQL LIKE statement fails and IMCE incorrectly shows an empty directory when used with FileField_sources. Specifically, I am referring to line 199 in filefield_source_imce_custom_scan() in imce.inc:

$result = db_query("SELECT filename FROM {files} WHERE filepath LIKE '%s' AND filepath NOT LIKE '%s'", $sql_dir_name .'/%', $sql_dir_name .'/%/%');

This means that FileField_sources/IMCE integration is broken for those who use private downloads on Windows servers (because their file directory paths have backslashes that require escaping, eg. C:\some_file_path). There might be other occasions too where valid directory paths require escaping (and a possible security issue?).

Problem is solved by by using db_escape_string, ie. change line 197 to:

$sql_dir_name = $dirname == '.' ? db_escape_string(file_directory_path()) : db_escape_string(file_directory_path() .'/'. $dirname);

Comments

dazweeja’s picture

dazweeja commented

OK, digging into this some more it seems that it's not as straightforward as I first thought. db_query already runs db_escape_string on all strings ('%s') so I'm not sure why this issue exists. I do know that changing line 197 to that above fixed the problem for me but this looks to me like db_escape_string would be run twice in this situation (by db_query and filefield_source_imce_custom_scan) which is strange that that would fix the problem.

Could anyone else with a Windows server confirm the problem?

Steps to reproduce:

1. Install FileField Sources 6.x-1.2 (current recommended version)
2. Install IMCE 6.x-2.0-rc1 (current recommended version)
3. Set downloads to private with common Windows file path, eg. C:\some\file\path
4. Upload a test file using FileField.
5. Browse for file to attach using FileField Sources/IMCE integration.

In my case, directory containing file uploaded in step 4 displays as empty even though there is a file in there (as well as corresponding entry in 'files' table). Also, debugging shows that the SQL LIKE query (line 199 above) returns FALSE.

dazweeja’s picture

dazweeja commented
Title: IMCE integration shows directory as empty (should use db_escape_string) » IMCE integration shows directory as empty (backslashes in filepaths need to be escaped twice)

I've found the problem and the solution. This is unexpected but searching for literal backslashes in LIKE patterns requires them to be escaped twice in MySQL. From the MySQL docs:

To search for “\”, specify it as “\\\\”; this is because the backslashes are stripped once by the parser and again when the pattern match is made, leaving a single backslash to be matched against.

Same with PostgreSQL.

So instead I propose that line 197 in imce.inc becomes:

$sql_dir_name = $dirname == '.' ? file_directory_path() : file_directory_path() .'/'. $dirname;
// Matching literal backslashes requires them to be escaped twice, once here and again by db_query.
$sql_dir_name = str_replace('\\', '\\\\', $sql_dir_name);
quicksketch’s picture

quicksketch
he/him
commented
Title: IMCE integration shows directory as empty (backslashes in filepaths need to be escaped twice) » Backslashes in filepaths need to be escaped twice in IMCE Source
Component: Code » Source: IMCE
Priority: Normal » Minor

Hm, cripes. Um, probably won't fix. I'd recommend using Transliteration module clean up such file names anyway. Window computers can't even use backslashes in file names anyway.

dazweeja’s picture

dazweeja commented

Thanks for looking at this. It's not the filename, it's the filepath that is being compared with the LIKE operator. I'm not sure if you use Windows, but writing the name of a local directory as, for example, 'c:\windows\temp' is standard Windows notation, not a rare edge case. So if this is the directory configured for 'File system path:' when using private downloads, a typical filepath in the files directory is stored as 'c:\windows\temp/file.jpg'.

This module is the only one I've seen that has issues in this circumstance, as it's the only module that I have experience with that uses a LIKE comparison on the filepath column.

quicksketch’s picture

quicksketch
he/him
commented

Okay, so over a year later I'm taking a look at this again. I can't see why it would be needed because Drupal is already running all the strings in db_query() through http://api.drupal.org/api/search/6/db_escape_string (which eventually calls http://php.net/mysqli_real_escape_string, which will do this conversion for you. You're right that I don't use Windows computers for hosts often though, so I could be missing something here.

quicksketch’s picture

quicksketch
he/him
commented
Status: Active » Closed (cannot reproduce)

This *may* have been fixed by #877452: Enable uploading and deletion through IMCE, which touched on this code. But I can't reproduce it either way. If the problem *does* still exist, I'll probably take another look with a patch, otherwise I probably won't be spending time to fix this. Please reopen if needed.