SQL: Disallow implicit integer downconversion on BindInt

Calling sql::Statement::BindInt() with an int64_t argument silently
downconverted, potentially saving incorrect data to a database.
sql::Statement::BindInt64(int64_t) should be used instead.

Introduce an uncallable sql::Statement::BindInt(int64_t) overload to
fail the compile. This also catches calling BindInt with uint32_t. Fix
up various callers that were silently downconverting.

Bug: 908690
Change-Id: I1472ad5461c7e60bfc011def85a48bc9ca216734
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935607
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723469}

chrome/browser/media/history/media_history_playback_table.cc

@@ -72,8 +72,8 @@ bool MediaHistoryPlaybackTable::SavePlayback(
       "VALUES ((SELECT id FROM origin WHERE origin = ?), ?, ?, ?, ?, ?, ?)"));
   statement.BindString(0, watch_time.origin.spec());
   statement.BindString(1, watch_time.url.spec());
-  statement.BindInt(2, watch_time.cumulative_watch_time.InMilliseconds());
-  statement.BindInt(3, watch_time.last_timestamp.InMilliseconds());
+  statement.BindInt64(2, watch_time.cumulative_watch_time.InMilliseconds());
+  statement.BindInt64(3, watch_time.last_timestamp.InMilliseconds());
   statement.BindInt(4, watch_time.has_video);
   statement.BindInt(5, watch_time.has_audio);
   statement.BindInt64(6,

components/history/core/browser/download_database.cc

@@ -268,7 +268,7 @@ bool DownloadDatabase::MigrateHashHttpMethodAndGenerateGuids() {
       base::StringPrintf("UPDATE %s SET guid = ? WHERE id = ?", kDownloadsTable)
           .c_str()));
   while (select.Step()) {
-    uint32_t id = select.ColumnInt(0);
+    int id = select.ColumnInt(0);
     uint64_t r1 = base::RandUint64();
     uint64_t r2 = base::RandUint64();
     std::string guid = base::StringPrintf(
@@ -598,7 +598,7 @@ bool DownloadDatabase::UpdateDownload(const DownloadRow& data) {
   statement.BindString(column++, data.by_ext_name);
   statement.BindString(column++, data.etag);
   statement.BindString(column++, data.last_modified);
-  statement.BindInt(column++, DownloadIdToInt(data.id));
+  statement.BindInt64(column++, DownloadIdToInt(data.id));
 
   if (!statement.Run())
     return false;
@@ -667,7 +667,7 @@ bool DownloadDatabase::CreateDownload(const DownloadRow& info) {
             .c_str()));
 
     int column = 0;
-    statement_insert.BindInt(column++, DownloadIdToInt(info.id));
+    statement_insert.BindInt64(column++, DownloadIdToInt(info.id));
     statement_insert.BindString(column++, info.guid);
     BindFilePath(statement_insert, info.current_path, column++);
     BindFilePath(statement_insert, info.target_path, column++);
@@ -711,7 +711,7 @@ bool DownloadDatabase::CreateDownload(const DownloadRow& info) {
   {
     sql::Statement count_urls(GetDB().GetCachedStatement(SQL_FROM_HERE,
         "SELECT count(*) FROM downloads_url_chains WHERE id=?"));
-    count_urls.BindInt(0, info.id);
+    count_urls.BindInt64(0, info.id);
     if (count_urls.Step()) {
       bool corrupt_urls = count_urls.ColumnInt(0) > 0;
       if (corrupt_urls) {
@@ -729,7 +729,7 @@ bool DownloadDatabase::CreateDownload(const DownloadRow& info) {
                                  "(id, chain_index, url) "
                                  "VALUES (?, ?, ?)"));
   for (size_t i = 0; i < info.url_chain.size(); ++i) {
-    statement_insert_chain.BindInt(0, info.id);
+    statement_insert_chain.BindInt64(0, info.id);
     statement_insert_chain.BindInt(1, static_cast<int>(i));
     statement_insert_chain.BindString(2, info.url_chain[i].spec());
     if (!statement_insert_chain.Run()) {
@@ -758,7 +758,7 @@ void DownloadDatabase::RemoveDownload(DownloadId id) {
       SQL_FROM_HERE,
       base::StringPrintf("DELETE FROM %s WHERE id=?", kDownloadsTable)
           .c_str()));
-  downloads_statement.BindInt(0, id);
+  downloads_statement.BindInt64(0, id);
   if (!downloads_statement.Run()) {
     UMA_HISTOGRAM_ENUMERATION("Download.DatabaseMainDeleteError",
                               GetDB().GetErrorCode() & 0xff, 50);
@@ -771,7 +771,7 @@ void DownloadDatabase::RemoveDownload(DownloadId id) {
 void DownloadDatabase::RemoveDownloadURLs(DownloadId id) {
   sql::Statement urlchain_statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM downloads_url_chains WHERE id=?"));
-  urlchain_statement.BindInt(0, id);
+  urlchain_statement.BindInt64(0, id);
   if (!urlchain_statement.Run()) {
     UMA_HISTOGRAM_ENUMERATION("Download.DatabaseURLChainDeleteError",
                               GetDB().GetErrorCode() & 0xff, 50);
@@ -803,7 +803,7 @@ bool DownloadDatabase::CreateOrUpdateDownloadSlice(
                          kDownloadsSlicesTable)
           .c_str()));
   int column = 0;
-  statement_replace.BindInt(column++, info.download_id);
+  statement_replace.BindInt64(column++, info.download_id);
   statement_replace.BindInt64(column++, info.offset);
   statement_replace.BindInt64(column++, info.received_bytes);
   statement_replace.BindInt64(column++, (info.finished ? 1 : 0));
@@ -815,7 +815,7 @@ void DownloadDatabase::RemoveDownloadSlices(DownloadId id) {
       SQL_FROM_HERE, base::StringPrintf("DELETE FROM %s WHERE download_id=?",
                                         kDownloadsSlicesTable)
                          .c_str()));
-  statement_delete.BindInt(0, id);
+  statement_delete.BindInt64(0, id);
   statement_delete.Run();
 }
 

components/history/core/browser/history_database.cc

@@ -260,11 +260,11 @@ int HistoryDatabase::CountUniqueDomainsVisited(base::Time begin_time,
                                                 // KEYWORD_GENERATED
       "AND hidden = 0 AND visit_time >= ? AND visit_time < ?"));
 
-  url_sql.BindInt(0, ui::PAGE_TRANSITION_CHAIN_END);
-  url_sql.BindInt(1, ui::PAGE_TRANSITION_CORE_MASK);
-  url_sql.BindInt(2, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  url_sql.BindInt(3, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
-  url_sql.BindInt(4, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
+  url_sql.BindInt64(0, ui::PAGE_TRANSITION_CHAIN_END);
+  url_sql.BindInt64(1, ui::PAGE_TRANSITION_CORE_MASK);
+  url_sql.BindInt64(2, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  url_sql.BindInt64(3, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
+  url_sql.BindInt64(4, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
 
   url_sql.BindInt64(5, begin_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
   url_sql.BindInt64(6, end_time.ToDeltaSinceWindowsEpoch().InMicroseconds());

components/history/core/browser/visit_database.cc

@@ -284,11 +284,11 @@ bool VisitDatabase::GetVisibleVisitsForURL(URLID url_id,
   statement.BindInt64(0, url_id);
   statement.BindInt64(1, options.EffectiveBeginTime());
   statement.BindInt64(2, options.EffectiveEndTime());
-  statement.BindInt(3, ui::PAGE_TRANSITION_CHAIN_END);
-  statement.BindInt(4, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(5, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  statement.BindInt(6, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
-  statement.BindInt(7, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
+  statement.BindInt64(3, ui::PAGE_TRANSITION_CHAIN_END);
+  statement.BindInt64(4, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(5, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  statement.BindInt64(6, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
+  statement.BindInt64(7, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
 
   return FillVisitVectorWithOptions(statement, options, visits);
 }
@@ -349,8 +349,8 @@ bool VisitDatabase::GetVisitsInRangeForTransition(base::Time begin_time,
   int64_t end = end_time.ToInternalValue();
   statement.BindInt64(0, begin_time.ToInternalValue());
   statement.BindInt64(1, end ? end : std::numeric_limits<int64_t>::max());
-  statement.BindInt(2, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(3, transition);
+  statement.BindInt64(2, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(3, transition);
   statement.BindInt64(
       4, max_results ? max_results : std::numeric_limits<int64_t>::max());
 
@@ -364,8 +364,8 @@ bool VisitDatabase::GetAllURLIDsForTransition(ui::PageTransition transition,
   sql::Statement statement(
       GetDB().GetUniqueStatement("SELECT DISTINCT url FROM visits "
                                  "WHERE (transition & ?) == ?"));
-  statement.BindInt(0, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(1, transition);
+  statement.BindInt64(0, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(1, transition);
 
   while (statement.Step()) {
     urls->push_back(statement.ColumnInt64(0));
@@ -390,11 +390,11 @@ bool VisitDatabase::GetVisibleVisitsInRange(const QueryOptions& options,
 
   statement.BindInt64(0, options.EffectiveBeginTime());
   statement.BindInt64(1, options.EffectiveEndTime());
-  statement.BindInt(2, ui::PAGE_TRANSITION_CHAIN_END);
-  statement.BindInt(3, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  statement.BindInt(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
-  statement.BindInt(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
+  statement.BindInt64(2, ui::PAGE_TRANSITION_CHAIN_END);
+  statement.BindInt64(3, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  statement.BindInt64(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
+  statement.BindInt64(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
 
   return FillVisitVectorWithOptions(statement, options, visits);
 }
@@ -447,7 +447,7 @@ bool VisitDatabase::GetRedirectFromVisit(VisitID from_visit,
       "WHERE v.from_visit = ? "
       "AND (v.transition & ?) != 0"));  // IS_REDIRECT_MASK
   statement.BindInt64(0, from_visit);
-  statement.BindInt(1, ui::PAGE_TRANSITION_IS_REDIRECT_MASK);
+  statement.BindInt64(1, ui::PAGE_TRANSITION_IS_REDIRECT_MASK);
 
   if (!statement.Step())
     return false;  // No redirect from this visit. (Or SQL error)
@@ -515,11 +515,11 @@ bool VisitDatabase::GetVisibleVisitCountToHost(const GURL& url,
   statement.BindString(0, host_query_min);
   statement.BindString(
       1, host_query_min.substr(0, host_query_min.size() - 1) + '0');
-  statement.BindInt(2, ui::PAGE_TRANSITION_CHAIN_END);
-  statement.BindInt(3, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  statement.BindInt(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
-  statement.BindInt(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
+  statement.BindInt64(2, ui::PAGE_TRANSITION_CHAIN_END);
+  statement.BindInt64(3, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  statement.BindInt64(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
+  statement.BindInt64(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
 
   if (!statement.Step()) {
     // We've never been to this page before.
@@ -556,11 +556,11 @@ bool VisitDatabase::GetHistoryCount(const base::Time& begin_time,
 
   statement.BindInt64(0, base::Time::kTimeTToMicrosecondsOffset);
   statement.BindInt64(1, base::Time::kMicrosecondsPerSecond);
-  statement.BindInt(2, ui::PAGE_TRANSITION_CHAIN_END);
-  statement.BindInt(3, ui::PAGE_TRANSITION_CORE_MASK);
-  statement.BindInt(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  statement.BindInt(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
-  statement.BindInt(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
+  statement.BindInt64(2, ui::PAGE_TRANSITION_CHAIN_END);
+  statement.BindInt64(3, ui::PAGE_TRANSITION_CORE_MASK);
+  statement.BindInt64(4, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
+  statement.BindInt64(5, ui::PAGE_TRANSITION_MANUAL_SUBFRAME);
+  statement.BindInt64(6, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
   statement.BindInt64(7, begin_time.ToInternalValue());
   statement.BindInt64(8, end_time.ToInternalValue());
 

components/offline_pages/core/background/request_queue_store.cc

@@ -300,7 +300,7 @@ AddRequestResult InsertSync(sql::Database* db, const SavePageRequest& request) {
   statement.BindString(10, request.original_url().spec());
   statement.BindString(11, request.request_origin());
   statement.BindInt64(12, static_cast<int64_t>(request.fail_state()));
-  statement.BindInt(
+  statement.BindInt64(
       13, static_cast<int64_t>(request.auto_fetch_notification_state()));
 
   if (!statement.Run())

components/offline_pages/core/offline_page_metadata_store_unittest.cc

@@ -390,9 +390,9 @@ void InjectItemInM62Store(sql::Database* db, const OfflinePageItem& item) {
       "digest) "
       "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"));
   statement.BindInt64(0, item.offline_id);
-  statement.BindInt(1, store_utils::ToDatabaseTime(item.creation_time));
+  statement.BindInt64(1, store_utils::ToDatabaseTime(item.creation_time));
   statement.BindInt64(2, item.file_size);
-  statement.BindInt(3, store_utils::ToDatabaseTime(item.last_access_time));
+  statement.BindInt64(3, store_utils::ToDatabaseTime(item.last_access_time));
   statement.BindInt(4, item.access_count);
   statement.BindString(5, item.client_id.name_space);
   statement.BindString(6, item.client_id.id);
@@ -402,7 +402,7 @@ void InjectItemInM62Store(sql::Database* db, const OfflinePageItem& item) {
   statement.BindString(10, item.original_url_if_different.spec());
   statement.BindString(11, item.request_origin);
   statement.BindInt64(12, item.system_download_id);
-  statement.BindInt(13, store_utils::ToDatabaseTime(item.file_missing_time));
+  statement.BindInt64(13, store_utils::ToDatabaseTime(item.file_missing_time));
   statement.BindString(14, item.digest);
   ASSERT_TRUE(statement.Run());
   ASSERT_TRUE(db->CommitTransaction());

components/password_manager/core/browser/field_info_table.cc

@@ -85,8 +85,8 @@ bool FieldInfoTable::AddRow(const FieldInfo& field) {
       "VALUES (?, ?, ?, ?)"));
   s.BindInt64(GetColumnNumber(FieldInfoTableColumn::kFormSignature),
               field.form_signature);
-  s.BindInt(GetColumnNumber(FieldInfoTableColumn::kFieldSignature),
-            field.field_signature);
+  s.BindInt64(GetColumnNumber(FieldInfoTableColumn::kFieldSignature),
+              field.field_signature);
   s.BindInt(GetColumnNumber(FieldInfoTableColumn::kFieldType),
             field.field_type);
   s.BindInt64(GetColumnNumber(FieldInfoTableColumn::kCreateTime),

sql/statement.h

@@ -106,6 +106,7 @@ class COMPONENT_EXPORT(SQL) Statement {
   bool BindNull(int col);
   bool BindBool(int col, bool val);
   bool BindInt(int col, int val);
+  bool BindInt(int col, int64_t val) = delete;  // Call BindInt64() instead.
   bool BindInt64(int col, int64_t val);
   bool BindDouble(int col, double val);
   bool BindCString(int col, const char* val);