Bug 257315 caused some timestamps to no longer be quoted, which makes it impossible to create new bugs on landfill (among other things)

I think we don't need to use SqlQuote, because: -> $timestamp is already trick_tainted (by not been derived from user input) -> it contains digits and safe chars, so calling dbh->quote on it is not required. -> simply adding quotes around it makes the solution simple and elegant, although placeholders would be nicer.

Comment on attachment 172754 [details] [diff] [review] patch v1 > "VALUES($bugid, $whoid, $timestamp, " . SqlQuote($comment) . ", " . > $privacyval . ", " . SqlQuote($work_time) . ")"); > >- SendSQL("UPDATE bugs SET delta_ts = $timestamp WHERE bug_id = $bugid"); >+ SendSQL("UPDATE bugs SET delta_ts = '$timestamp' WHERE bug_id = $bugid"); This $timestamp is already quoted, above, unless I'm mistaken: $timestamp = ($timestamp ? SqlQuote($timestamp) : "NOW()"); However, the SqlQuote should probably be moved outside of the ternary so that it also covers the NOW(). >-$sql .= "$::userid, $timestamp, $timestamp, "; >+$sql .= "$::userid, '$timestamp', '$timestamp', "; Yeah, and it's OK to not SqlQuote this one because it comes from the DB. >- SendSQL("UPDATE bugs SET delta_ts = $timestamp," . >+ SendSQL("UPDATE bugs SET delta_ts = '$timestamp'," . Same for this one (it's the same variable, even). >- SendSQL("UPDATE bugs SET delta_ts=$timestamp WHERE bug_id=$i"); >+ SendSQL("UPDATE bugs SET delta_ts='$timestamp' WHERE bug_id=$i"); Technically, this one is passed into the function from an arbitrary location, so *maybe* it should be SqlQuoted. But it wasn't before, so this is probably OK. We'll just get a taint error if this function gets a tainted timestamp, which is probably fine. >- SendSQL("UPDATE bugs SET delta_ts = $timestamp, keywords = " . >+ SendSQL("UPDATE bugs SET delta_ts = '$timestamp', keywords = " . And this one also comes from the DB, so we're OK. So only the first one needs to be fixed.

Bugger, good catch. I need to spend more time testing it next time, sorry :-(. I went through the whole patch which caused this and found a few more places where quotes are missing. I have also converted few SqlQuote() calls to simpe quotes after verifying that they are coming directly from the DB (even in functions, if all calls are getting the passed parameter from the DB).

Comment on attachment 172788 [details] [diff] [review] V2 You know, actually, I'd rather you leave any SqlQuotes there that are already being done. They don't hurt, and they take very little time. Just quote the values that are current unquoted and cause Bugzilla to break.

*** This bug has been marked as a duplicate of 257315 ***

clearing target of DUPLICATE/WONTFIX/INVALID/WORKSFORME so they'll show up as untriaged if they get reopened.