From 8664b9ad609c1661d97a3145ce501c6df0e2bbb1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Mon, 22 Jul 2024 14:22:17 +0200
Subject: [PATCH] src,test: disallow unsafe integer coercion in SQLite

Currently, by default (i.e., when use_big_ints_ has not explicitly been
set to true), reading a SQLite integer value that is not a safe integer
in JavaScript is likely to yield an incorrect number.

Instead, err on the side of caution and throw if the stored integer is
not a safe integer in JavaScript and if use_big_ints_ has not been set
to true.

PR-URL: https://github.com/nodejs/node/pull/53851
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 src/node_sqlite.cc           | 17 +++++++++++++----
 test/parallel/test-sqlite.js | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 1202d2c8cf2464..fbef4e3b0d71c8 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -370,12 +370,21 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
 
 Local<Value> StatementSync::ColumnToValue(const int column) {
   switch (sqlite3_column_type(statement_, column)) {
-    case SQLITE_INTEGER:
+    case SQLITE_INTEGER: {
+      sqlite3_int64 value = sqlite3_column_int64(statement_, column);
       if (use_big_ints_) {
-        return BigInt::New(env()->isolate(),
-                           sqlite3_column_int64(statement_, column));
+        return BigInt::New(env()->isolate(), value);
+      } else if (std::abs(value) <= kMaxSafeJsInteger) {
+        return Number::New(env()->isolate(), value);
+      } else {
+        THROW_ERR_OUT_OF_RANGE(env()->isolate(),
+                               "The value of column %d is too large to be "
+                               "represented as a JavaScript number: %" PRId64,
+                               column,
+                               value);
+        return Local<Value>();
       }
-      // Fall through.
+    }
     case SQLITE_FLOAT:
       return Number::New(env()->isolate(),
                          sqlite3_column_double(statement_, column));
diff --git a/test/parallel/test-sqlite.js b/test/parallel/test-sqlite.js
index 99c8b7ee72a5c4..d07c3ac01b9a23 100644
--- a/test/parallel/test-sqlite.js
+++ b/test/parallel/test-sqlite.js
@@ -388,6 +388,22 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
       message: /The "readBigInts" argument must be a boolean/,
     });
   });
+
+  test('BigInt is required for reading large integers', (t) => {
+    const db = new DatabaseSync(nextDb());
+    const bad = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
+    t.assert.throws(() => {
+      bad.get();
+    }, {
+      code: 'ERR_OUT_OF_RANGE',
+      message: /^The value of column 0 is too large.*: 9007199254740992$/,
+    });
+    const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
+    good.setReadBigInts(true);
+    t.assert.deepStrictEqual(good.get(), {
+      [`${Number.MAX_SAFE_INTEGER} + 1`]: 2n ** 53n,
+    });
+  });
 });
 
 suite('StatementSync.prototype.setAllowBareNamedParameters()', () => {