Skip to content

Commit 2714896

Browse files
committed
sqlite: restrict valid authz code
1 parent f63cba7 commit 2714896

File tree

2 files changed

+40
-13
lines changed

2 files changed

+40
-13
lines changed

src/node_sqlite.cc

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1930,7 +1930,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data,
19301930

19311931
if (try_catch.HasCaught()) {
19321932
// If there's an exception in the callback, deny the operation
1933-
// TODO: Rethrow exepction
1933+
// TODO: Rethrow exception
19341934
return SQLITE_DENY;
19351935
}
19361936

@@ -1939,13 +1939,19 @@ int DatabaseSync::AuthorizerCallback(void* user_data,
19391939
return SQLITE_DENY;
19401940
}
19411941

1942-
if (result->IsNumber()) {
1943-
double num_result = result.As<Number>()->Value();
1944-
return static_cast<int>(num_result);
1942+
if (!result->IsNumber()) {
1943+
return SQLITE_DENY;
1944+
}
1945+
1946+
double num_result = result.As<Number>()->Value();
1947+
int int_result = static_cast<int>(num_result);
1948+
1949+
if (int_result == SQLITE_OK || int_result == SQLITE_DENY ||
1950+
int_result == SQLITE_IGNORE) {
1951+
return int_result;
19451952
}
19461953

1947-
// Default to OK if the result isn't a number
1948-
return SQLITE_OK;
1954+
return SQLITE_DENY;
19491955
}
19501956

19511957
StatementSync::StatementSync(Environment* env,

test/parallel/test-sqlite-authz.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,34 @@ suite('DatabaseSync.prototype.setAuthorizer()', () => {
8585
});
8686
});
8787

88+
it('blocks operations when authorizer returns NaN', () => {
89+
const db = new DatabaseSync(':memory:');
90+
db.setAuthorizer(() => {
91+
return '1';
92+
});
93+
94+
assert.throws(() => {
95+
db.exec('SELECT 1');
96+
}, {
97+
code: 'ERR_SQLITE_ERROR',
98+
message: /not authorized/
99+
});
100+
});
101+
102+
it('blocks operations when authorizer returns a invalid code', () => {
103+
const db = new DatabaseSync(':memory:');
104+
db.setAuthorizer(() => {
105+
return 3;
106+
});
107+
108+
assert.throws(() => {
109+
db.exec('SELECT 1');
110+
}, {
111+
code: 'ERR_SQLITE_ERROR',
112+
message: /not authorized/
113+
});
114+
});
115+
88116
it('clears authorizer when set to null', (t) => {
89117
const authorizer = t.mock.fn(() => constants.SQLITE_OK);
90118
const db = new DatabaseSync(':memory:');
@@ -155,11 +183,4 @@ suite('DatabaseSync.prototype.setAuthorizer()', () => {
155183
message: /The "callback" argument must be a function/
156184
});
157185
});
158-
159-
it('accepts null as valid input for clearing authorizer', () => {
160-
const db = new DatabaseSync(':memory:');
161-
162-
// does not throw
163-
db.setAuthorizer(null);
164-
});
165186
});

0 commit comments

Comments
 (0)