Mtasa-blue: dbPrepareString replaces ? and ?? with single quotes

[moon91210]

Describe the bug

Just like the title says.

To reproduce

local q = con:prepareString('UPDATE users SET `??`=? WHERE `??`=?',
    'nickname',
    'slimshady???',
    'id',
    1
)
con:query(q)

Expected behaviour

I'm not entirely sure if this is done on purpose but I was expecting this in the database:

slimshady???

Reality:

slimshady''

Should I refrain from using dbPrepareString when using question marks in strings?

I thought this function was adding an extra safety measure to prevent sql injections from user input. I didn't expect this behaviour.

Screenshots

Version
1.5.6, win7

[flasherr]

This query is correctly:

local q = con:prepareString(
    "UPDATE ?? SET ??=? WHERE ??=?;", 
    "users", "nickname", "slimshady???", "id", 1
)
con:query(q)

and should be working. It isn't bug, you don't know how to use prepare string.

[moon91210]

The query is fine. And nope.

[Addlibs]

Can confirm this as a bug.

The given prepare statement generates this SQL statement

UPDATE users SET `nickname`='slimshady???' WHERE `id`=1

which includes ??? and statements are reevaluated again when dbQuery or dbExec is called, causing question marks to be replaced with two single-quotes (which turns into a single-quote or apostrophe). I.e. the actual statement that gets executed is

UPDATE users SET `nickname`='slimshady''' WHERE `id`=1

This doesn't seem to raise an error per se, but changes the results of the query (slimshady doesn't get fetched if you query for slimshady'', but slimshady' does). Additionally, I can imagine someone will find a way to inject bad stuff through that.

I believe it would be easy to fix just by not reevaluating question marks in the statement if no substitution parameters are given in dbQuery or dbExec.

[ccw808]

@Addlibs thanks for clarifying the issue. I think the best solution would be to not replace ? when they are already inside quotes.

[rambolger]

Sadly, this change broke a few of our queries - especially when ?? is used together with % wildcarding. It is not possible to combine ? or ?? with wildcard symbols anymore.

[ccw808]

@rambolger Please give an example query

[rambolger]

SELECT id, nickname, userid, serial, ip, dateline, banstatus, reason, admin, adminuserid FROM bans WHERE userid LIKE '??' or serial LIKE '??' or LOWER(nickname) LIKE '??%' or LOWER(nickname) LIKE '%??%' or ip LIKE '??%' or ip LIKE '%??%' ORDER BY id DESC

[CrosRoad95]

Why mta using ?? and ? instead of using format string patterns ?

[ccw808]

@rambolger thanks, although for injection protection you should be using ? instead of '%??%' and apply modifications to the input string i.e. "%"..name.."%"

[rambolger]

I tried using ? together with "%"..name.."%" and it still did not work with build 14687.

[rambolger]

I suppose what ccw was talking about is the following:

local strUserInput = "A%' OR 1 = 1; DROP TABLE bla; -- "
local strQueryStringPrepared = dbPrepareString(uConnection, "SELECT * FROM t WHERE `val` LIKE ?;", "%" .. strUserInput .. "%")

[ghost]

@rambolger yeah, you're right, dbPrepareString is working as expected - theres nothing wrong with it as it only escapes \x00, \n, \r, \, ', " and \x1a only

?? is used for unquoted strings which is also working as expected.

• Luxy.c

[ccw808]

@moon91210 Fix is in build 14689 and higher - Please try with latest from https://nightly.mtasa.com/