FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sqlite: invalidate tag store iterators on statement reset · nodejs/node@af9ff04 · GitHub

/ node Public

Commit af9ff04

Browse files
mcollina
authored and
RafaelGSS
committed
sqlite: invalidate tag store iterators on statement reset
SQLTagStore Run/Iterate/Get/All reset the shared cached statement via raw sqlite3_reset(), bypassing ResetStatement() and its reset_generation_ bump, so live iterators were never invalidated. Also mark the iterator done on SQLITE_DONE so it cannot restart the statement. PR-URL: nodejs-private/node-private#896 Refs: https://hackerone.com/reports/3564941 Reviewed-By: Robert Nagy <ronagy@icloud.com> CVE-ID: CVE-2026-58041
1 parent 95ba2cf commit af9ff04

2 files changed

Lines changed: 113 additions & 4 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3349,7 +3349,7 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {
33493349
}
33503350

33513351
uint32_t n_params = args.Length() - 1;
3352-
int r = sqlite3_reset(stmt->statement_);
3352+
int r = stmt->ResetStatement();
33533353
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
33543354
int param_count = sqlite3_bind_parameter_count(stmt->statement_);
33553355
for (int i = 0; i < static_cast<int>(n_params) && i < param_count; ++i) {
@@ -3382,7 +3382,7 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
33823382
}
33833383

33843384
uint32_t n_params = args.Length() - 1;
3385-
int r = sqlite3_reset(stmt->statement_);
3385+
int r = stmt->ResetStatement();
33863386
CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
33873387
int param_count = sqlite3_bind_parameter_count(stmt->statement_);
33883388
for (int i = 0; i < static_cast<int>(n_params) && i < param_count; ++i) {
@@ -3419,7 +3419,7 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
34193419
uint32_t n_params = args.Length() - 1;
34203420
Isolate* isolate = env->isolate();
34213421

3422-
int r = sqlite3_reset(stmt->statement_);
3422+
int r = stmt->ResetStatement();
34233423
CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void());
34243424

34253425
int param_count = sqlite3_bind_parameter_count(stmt->statement_);
@@ -3458,7 +3458,7 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
34583458
uint32_t n_params = args.Length() - 1;
34593459
Isolate* isolate = env->isolate();
34603460

3461-
int r = sqlite3_reset(stmt->statement_);
3461+
int r = stmt->ResetStatement();
34623462
CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void());
34633463

34643464
int param_count = sqlite3_bind_parameter_count(stmt->statement_);
@@ -3702,6 +3702,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
37023702
CHECK_ERROR_OR_THROW(
37033703
env->isolate(), iter->stmt_->db_.get(), r, SQLITE_DONE, void());
37043704
sqlite3_reset(iter->stmt_->statement_);
3705+
iter->done_ = true;
37053706
MaybeLocal<Value> values[] = {Boolean::New(isolate, true), Null(isolate)};
37063707
Local<Object> result;
37073708
if (NewDictionaryInstanceNullProto(env->context(), iter_template, values)

‎test/parallel/test-sqlite-template-tag.js‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,114 @@ test('TagStore capacity, size, and clear', () => {
112112
assert.strictEqual(sql.capacity, 10);
113113
});
114114

115+
test('iterator is invalidated when the cached statement is reset', () => {
116+
const ldb = new DatabaseSync(':memory:');
117+
const lsql = ldb.createTagStore();
118+
ldb.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY, text TEXT)');
119+
for (let i = 0; i < 5; i++) {
120+
// eslint-disable-next-line no-unused-expressions
121+
lsql.run`INSERT INTO foo (text) VALUES (${String(i)})`;
122+
}
123+
124+
// Invalidated by sql.get on the same tagged literal
125+
let it = lsql.iterate`SELECT * FROM foo ORDER BY id ASC`;
126+
it.next();
127+
// eslint-disable-next-line no-unused-expressions
128+
lsql.get`SELECT * FROM foo ORDER BY id ASC`;
129+
assert.throws(() => { it.next(); }, {
130+
code: 'ERR_INVALID_STATE',
131+
message: /iterator was invalidated/,
132+
});
133+
134+
// Invalidated by sql.all on the same tagged literal
135+
it = lsql.iterate`SELECT * FROM foo ORDER BY id ASC`;
136+
it.next();
137+
// eslint-disable-next-line no-unused-expressions
138+
lsql.all`SELECT * FROM foo ORDER BY id ASC`;
139+
assert.throws(() => { it.next(); }, {
140+
code: 'ERR_INVALID_STATE',
141+
message: /iterator was invalidated/,
142+
});
143+
144+
// Invalidated by sql.run on the same tagged literal
145+
it = lsql.iterate`SELECT * FROM foo ORDER BY id ASC`;
146+
it.next();
147+
// eslint-disable-next-line no-unused-expressions
148+
lsql.run`SELECT * FROM foo ORDER BY id ASC`;
149+
assert.throws(() => { it.next(); }, {
150+
code: 'ERR_INVALID_STATE',
151+
message: /iterator was invalidated/,
152+
});
153+
154+
// Invalidated by a new sql.iterate on the same tagged literal
155+
it = lsql.iterate`SELECT * FROM foo ORDER BY id ASC`;
156+
it.next();
157+
const it2 = lsql.iterate`SELECT * FROM foo ORDER BY id ASC`;
158+
assert.throws(() => { it.next(); }, {
159+
code: 'ERR_INVALID_STATE',
160+
message: /iterator was invalidated/,
161+
});
162+
163+
// The fresh iterator still works.
164+
assert.strictEqual(it2.next().done, false);
165+
166+
ldb.close();
167+
});
168+
169+
test('a stale iterator cannot replay a victim-bound write', () => {
170+
const bank = new DatabaseSync(':memory:');
171+
const tx = bank.createTagStore();
172+
bank.exec(`
173+
CREATE TABLE acct(user TEXT PRIMARY KEY, balance INTEGER);
174+
CREATE TABLE transfers(id INTEGER PRIMARY KEY AUTOINCREMENT,
175+
from_user TEXT, to_user TEXT, amount INTEGER);
176+
CREATE TRIGGER debit AFTER INSERT ON transfers
177+
BEGIN
178+
UPDATE acct SET balance = balance - NEW.amount WHERE user = NEW.from_user;
179+
UPDATE acct SET balance = balance + NEW.amount WHERE user = NEW.to_user;
180+
END;
181+
INSERT INTO acct VALUES ('victim', 1000);
182+
INSERT INTO acct VALUES ('attacker', 0);
183+
`);
184+
185+
// Attacker arms an iterator without stepping it. The static parts of this
186+
// tagged literal must be byte-identical to the victim's below so they share
187+
// the same cached statement.
188+
const armed = tx.iterate`INSERT INTO transfers(from_user, to_user, amount) VALUES (${'attacker'}, ${'attacker'}, ${0}) RETURNING id`;
189+
190+
// Victim makes one legitimate transfer through the same tagged literal.
191+
// eslint-disable-next-line no-unused-expressions
192+
tx.get`INSERT INTO transfers(from_user, to_user, amount) VALUES (${'victim'}, ${'attacker'}, ${100}) RETURNING id`;
193+
194+
// The stale iterator must not be able to replay the victim's write.
195+
assert.throws(() => { armed.next(); }, {
196+
code: 'ERR_INVALID_STATE',
197+
message: /iterator was invalidated/,
198+
});
199+
200+
const balances = bank.prepare('SELECT user, balance FROM acct ORDER BY user').all();
201+
assert.deepStrictEqual(balances, [
202+
{ __proto__: null, user: 'attacker', balance: 100 },
203+
{ __proto__: null, user: 'victim', balance: 900 },
204+
]);
205+
const count = bank.prepare('SELECT COUNT(*) AS c FROM transfers').get().c;
206+
assert.strictEqual(count, 1);
207+
208+
bank.close();
209+
});
210+
211+
test('a finished iterator stays done and does not restart', () => {
212+
// eslint-disable-next-line no-unused-expressions
213+
sql.run`INSERT INTO foo (text) VALUES (${'bob'})`;
214+
215+
const iter = sql.iterate`SELECT * FROM foo ORDER BY id ASC`;
216+
assert.strictEqual(iter.next().done, false);
217+
assert.strictEqual(iter.next().done, true);
218+
// A subsequent next() must not restart the statement.
219+
assert.strictEqual(iter.next().done, true);
220+
assert.strictEqual(iter.next().done, true);
221+
});
222+
115223
test('sql.db returns the associated DatabaseSync instance', () => {
116224
assert.strictEqual(sql.db, db);
117225
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL