Calls to db_lock_table in bootstrap.inc does not use db_prefix [#54981]

I wanted to test a multi-site setup by making copies of the variable-table and prefixing one for each site. This causes errors related to locking the variable-table (no prefix) when editing settings on any of the sites.

The code that causes this behavior is in includes/bootstrap.inc line 237 and 324

This has previously been discussed here: http://drupal.org/node/47135

I think hitka's proposed solution is ok, simply add the prefix before calling db_lock_table, but maybe this should be handled in database.*.inc.

Comment	File	Size	Author
#10	lock.table.prefix.patch	2.41 KB	Steven
#10
#9	db_lock_table_3.patch	1.67 KB	rkerr
#9
#8	db_lock_table_2.patch	1.72 KB	rkerr
#8
#7	db_lock_table.patch	1.07 KB	rkerr
#7

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

arkepp CreditAttribution: arkepp commented 20 March 2006 at 19:34

Just a small clarification: If you keep the original table named 'variable' you do not get any error messages, but that of course means you lock and unlock the wrong table.

Comment #2

wulff CreditAttribution: wulff commented 19 April 2006 at 08:01

AFAICT this has been fixed in cvs: db_lock_table() calls db_query() which takes care of prefixing the tables before running the query.

Comment #3

olav CreditAttribution: olav commented 27 April 2006 at 10:55

Version:	4.7.0-beta6	» 4.7.0-rc3
Component:	base system	» database system
Assigned:	Unassigned	» olav
Status:	Active	» Needs review

4.7rc3 still has this bug.
The fix is simple:

Replace (database.mysql.inc:356)

db_query('LOCK TABLES {%s} WRITE', $table);

with

db_query('LOCK TABLES {' . $table . '} WRITE');

--
Olav

Comment #4

Gerhard Killesreiter CreditAttribution: Gerhard Killesreiter commented 27 April 2006 at 15:11

Version:	4.7.0-rc3	» x.y.z
Priority:	Normal	» Critical

Olav, can you explain why

db_query('LOCK TABLES {%s} WRITE', $table);

does not work?

Comment #5

rkerr CreditAttribution: rkerr commented 27 April 2006 at 16:12

Must be in how db_query stuff parses arguments.
I'll bet that anything that gets sprintf'd into the query string is not checked for {table_name} and run through the prefixing routine.

Comment #6

moshe weitzman CreditAttribution: moshe weitzman commented 27 April 2006 at 18:18

#3 looks safe to me. nohere do we use user generated input as a table name.

Comment #7

rkerr CreditAttribution: rkerr commented 27 April 2006 at 19:45

File	Size
db_lock_table.patch	1.07 KB

Patch attached. For MySQL and MySQLi as well.

Comment #8

rkerr CreditAttribution: rkerr commented 27 April 2006 at 19:59

File	Size
db_lock_table_2.patch	1.72 KB

Added db_escape_string around $table for safety, and updated PostgreSQL inc file as well.
(New patch attached).

Comment #9

rkerr CreditAttribution: rkerr commented 27 April 2006 at 20:07

File	Size
db_lock_table_3.patch	1.67 KB

Another patch for all 3 database includes, without escaping.

Comment #10

Steven CreditAttribution: Steven commented 27 April 2006 at 20:10

File	Size
lock.table.prefix.patch	2.41 KB

Using db_escape_string here is useless. The characters are not inserted inside an SQL string, but inside the SQL query itself. You don't need to get out of a " to do bad things.

Using %s is bad because prefixing is done before inserting the data. If you have a table-specific prefix, it will not get applied correctly.

It's safer to restrict table vars using an explicit db_escape_table() function which only allows alphanumerics. Patch attached.