Skip to content

Commit

Permalink
patch SQL injection opportunities [LOW RISK] (#234)
Browse files Browse the repository at this point in the history
  • Loading branch information
timothycarambat committed Sep 11, 2023
1 parent 3c88aec commit dc3dfbf
Show file tree
Hide file tree
Showing 13 changed files with 69 additions and 41 deletions.
18 changes: 11 additions & 7 deletions server/endpoints/admin.js
@@ -1,3 +1,4 @@
const { escape } = require("sqlstring-sqlite");
const { ApiKey } = require("../models/apiKeys");
const { Document } = require("../models/documents");
const { Invite } = require("../models/invite");
Expand Down Expand Up @@ -203,7 +204,7 @@ function adminEndpoints(app) {
const { workspaceId } = request.params;
const { userIds } = reqBody(request);
const { success, error } = await Workspace.updateUsers(
workspaceId,
escape(Number(workspaceId)),
userIds
);
response.status(200).json({ success, error });
Expand All @@ -227,16 +228,16 @@ function adminEndpoints(app) {

const { id } = request.params;
const VectorDb = getVectorDbClass();
const workspace = Workspace.get(`id = ${id}`);
const workspace = Workspace.get(`id = ${escape(id)}`);
if (!workspace) {
response.sendStatus(404).end();
return;
}

await Workspace.delete(`id = ${id}`);
await DocumentVectors.deleteForWorkspace(id);
await Document.delete(`workspaceId = ${Number(id)}`);
await WorkspaceChats.delete(`workspaceId = ${Number(id)}`);
await Workspace.delete(`id = ${workspace.id}`);
await DocumentVectors.deleteForWorkspace(workspace.id);
await Document.delete(`workspaceId = ${Number(workspace.id)}`);
await WorkspaceChats.delete(`workspaceId = ${Number(workspace.id)}`);
try {
await VectorDb["delete-namespace"]({ namespace: workspace.slug });
} catch (e) {
Expand All @@ -262,7 +263,10 @@ function adminEndpoints(app) {
return;
}
const { offset = 0 } = reqBody(request);
const chats = await WorkspaceChats.whereWithData(`id >= ${offset}`, 20);
const chats = await WorkspaceChats.whereWithData(
`id >= ${escape(offset)}`,
20
);
const hasPages = (await WorkspaceChats.count()) > 20;
response.status(200).json({ chats: chats.reverse(), hasPages });
} catch (e) {
Expand Down
8 changes: 6 additions & 2 deletions server/endpoints/api/admin/index.js
@@ -1,3 +1,4 @@
const { escape } = require("sqlstring-sqlite");
const { Invite } = require("../../../models/invite");
const { SystemSettings } = require("../../../models/systemSettings");
const { User } = require("../../../models/user");
Expand Down Expand Up @@ -456,7 +457,7 @@ function apiAdminEndpoints(app) {
const { workspaceId } = request.params;
const { userIds } = reqBody(request);
const { success, error } = await Workspace.updateUsers(
workspaceId,
escape(Number(workspaceId)),
userIds
);
response.status(200).json({ success, error });
Expand Down Expand Up @@ -515,7 +516,10 @@ function apiAdminEndpoints(app) {
}

const { offset = 0 } = reqBody(request);
const chats = await WorkspaceChats.whereWithData(`id >= ${offset}`, 20);
const chats = await WorkspaceChats.whereWithData(
`id >= ${escape(offset)}`,
20
);
const hasPages = (await WorkspaceChats.count()) > 20;
response.status(200).json({ chats: chats.reverse(), hasPages });
} catch (e) {
Expand Down
17 changes: 10 additions & 7 deletions server/endpoints/api/workspace/index.js
@@ -1,3 +1,4 @@
const { escape } = require("sqlstring-sqlite");
const { Document } = require("../../../models/documents");
const { Telemetry } = require("../../../models/telemetry");
const { DocumentVectors } = require("../../../models/vectors");
Expand Down Expand Up @@ -153,7 +154,7 @@ function apiWorkspaceEndpoints(app) {
*/
try {
const { slug } = request.params;
const workspace = await Workspace.get(`slug = '${slug}'`);
const workspace = await Workspace.get(`slug = ${escape(slug)}`);
response.status(200).json({ workspace });
} catch (e) {
console.log(e.message, e);
Expand Down Expand Up @@ -184,14 +185,14 @@ function apiWorkspaceEndpoints(app) {
try {
const { slug = "" } = request.params;
const VectorDb = getVectorDbClass();
const workspace = await Workspace.get(`slug = '${slug}'`);
const workspace = await Workspace.get(`slug = ${escape(slug)}`);

if (!workspace) {
response.sendStatus(400).end();
return;
}

await Workspace.delete(`slug = '${slug.toLowerCase()}'`);
await Workspace.delete(`id = ${Number(workspace.id)}`);
await DocumentVectors.deleteForWorkspace(workspace.id);
await Document.delete(`workspaceId = ${Number(workspace.id)}`);
await WorkspaceChats.delete(`workspaceId = ${Number(workspace.id)}`);
Expand Down Expand Up @@ -269,7 +270,7 @@ function apiWorkspaceEndpoints(app) {
try {
const { slug = null } = request.params;
const data = reqBody(request);
const currWorkspace = await Workspace.get(`slug = '${slug}'`);
const currWorkspace = await Workspace.get(`slug = ${escape(slug)}`);

if (!currWorkspace) {
response.sendStatus(400).end();
Expand Down Expand Up @@ -333,7 +334,7 @@ function apiWorkspaceEndpoints(app) {
*/
try {
const { slug } = request.params;
const workspace = await Workspace.get(`slug = '${slug}'`);
const workspace = await Workspace.get(`slug = ${escape(slug)}`);

if (!workspace) {
response.sendStatus(400).end();
Expand Down Expand Up @@ -408,7 +409,7 @@ function apiWorkspaceEndpoints(app) {
try {
const { slug = null } = request.params;
const { adds = [], deletes = [] } = reqBody(request);
const currWorkspace = await Workspace.get(`slug = '${slug}'`);
const currWorkspace = await Workspace.get(`slug = ${escape(slug)}`);

if (!currWorkspace) {
response.sendStatus(400).end();
Expand All @@ -417,7 +418,9 @@ function apiWorkspaceEndpoints(app) {

await Document.removeDocuments(currWorkspace, deletes);
await Document.addDocuments(currWorkspace, adds);
const updatedWorkspace = await Workspace.get(`slug = '${slug}'`);
const updatedWorkspace = await Workspace.get(
`id = ${Number(currWorkspace.id)}`
);
response.status(200).json({ workspace: updatedWorkspace });
} catch (e) {
console.log(e.message, e);
Expand Down
5 changes: 3 additions & 2 deletions server/endpoints/chat.js
Expand Up @@ -6,6 +6,7 @@ const { validatedRequest } = require("../utils/middleware/validatedRequest");
const { WorkspaceChats } = require("../models/workspaceChats");
const { SystemSettings } = require("../models/systemSettings");
const { Telemetry } = require("../models/telemetry");
const { escape } = require("sqlstring-sqlite");

function chatEndpoints(app) {
if (!app) return;
Expand All @@ -19,8 +20,8 @@ function chatEndpoints(app) {
const { slug } = request.params;
const { message, mode = "query" } = reqBody(request);
const workspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

if (!workspace) {
response.sendStatus(400).end();
Expand Down
5 changes: 3 additions & 2 deletions server/endpoints/invite.js
@@ -1,3 +1,4 @@
const { escape } = require("sqlstring-sqlite");
const { Invite } = require("../models/invite");
const { User } = require("../models/user");
const { reqBody } = require("../utils/http");
Expand All @@ -8,7 +9,7 @@ function inviteEndpoints(app) {
app.get("/invite/:code", async (request, response) => {
try {
const { code } = request.params;
const invite = await Invite.get(`code = '${code}'`);
const invite = await Invite.get(`code = ${escape(code)}`);
if (!invite) {
response.status(200).json({ invite: null, error: "Invite not found." });
return;
Expand All @@ -34,7 +35,7 @@ function inviteEndpoints(app) {
try {
const { code } = request.params;
const userParams = reqBody(request);
const invite = await Invite.get(`code = '${code}'`);
const invite = await Invite.get(`code = ${escape(code)}`);
if (!invite || invite.status !== "pending") {
response
.status(200)
Expand Down
3 changes: 2 additions & 1 deletion server/endpoints/system.js
Expand Up @@ -38,6 +38,7 @@ const {
const { Telemetry } = require("../models/telemetry");
const { WelcomeMessages } = require("../models/welcomeMessages");
const { ApiKey } = require("../models/apiKeys");
const { escape } = require("sqlstring-sqlite");

function systemEndpoints(app) {
if (!app) return;
Expand Down Expand Up @@ -96,7 +97,7 @@ function systemEndpoints(app) {
try {
if (await SystemSettings.isMultiUserMode()) {
const { username, password } = reqBody(request);
const existingUser = await User.get(`username = '${username}'`);
const existingUser = await User.get(`username = ${escape(username)}`);

if (!existingUser) {
response.status(200).json({
Expand Down
27 changes: 15 additions & 12 deletions server/endpoints/workspaces.js
Expand Up @@ -13,6 +13,7 @@ const {
const { validatedRequest } = require("../utils/middleware/validatedRequest");
const { SystemSettings } = require("../models/systemSettings");
const { Telemetry } = require("../models/telemetry");
const { escape } = require("sqlstring-sqlite");
const { handleUploads } = setupMulter();

function workspaceEndpoints(app) {
Expand Down Expand Up @@ -44,8 +45,8 @@ function workspaceEndpoints(app) {
const { slug = null } = request.params;
const data = reqBody(request);
const currWorkspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

if (!currWorkspace) {
response.sendStatus(400).end();
Expand Down Expand Up @@ -105,8 +106,8 @@ function workspaceEndpoints(app) {
const { slug = null } = request.params;
const { adds = [], deletes = [] } = reqBody(request);
const currWorkspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

if (!currWorkspace) {
response.sendStatus(400).end();
Expand All @@ -115,7 +116,9 @@ function workspaceEndpoints(app) {

await Document.removeDocuments(currWorkspace, deletes);
await Document.addDocuments(currWorkspace, adds);
const updatedWorkspace = await Workspace.get(`slug = '${slug}'`);
const updatedWorkspace = await Workspace.get(
`id = ${currWorkspace.id}`
);
response.status(200).json({ workspace: updatedWorkspace });
} catch (e) {
console.log(e.message, e);
Expand All @@ -133,8 +136,8 @@ function workspaceEndpoints(app) {
const user = await userFromSession(request, response);
const VectorDb = getVectorDbClass();
const workspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

if (!workspace) {
response.sendStatus(400).end();
Expand All @@ -151,7 +154,7 @@ function workspaceEndpoints(app) {
}
}

await Workspace.delete(`slug = '${slug.toLowerCase()}'`);
await Workspace.delete(`id = ${Number(workspace.id)}`);
await DocumentVectors.deleteForWorkspace(workspace.id);
await Document.delete(`workspaceId = ${Number(workspace.id)}`);
await WorkspaceChats.delete(`workspaceId = ${Number(workspace.id)}`);
Expand Down Expand Up @@ -187,8 +190,8 @@ function workspaceEndpoints(app) {
const { slug } = request.params;
const user = await userFromSession(request, response);
const workspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

response.status(200).json({ workspace });
} catch (e) {
Expand All @@ -205,8 +208,8 @@ function workspaceEndpoints(app) {
const { slug } = request.params;
const user = await userFromSession(request, response);
const workspace = multiUserMode(response)
? await Workspace.getWithUser(user, `slug = '${slug}'`)
: await Workspace.get(`slug = '${slug}'`);
? await Workspace.getWithUser(user, `slug = ${escape(slug)}`)
: await Workspace.get(`slug = ${escape(slug)}`);

if (!workspace) {
response.sendStatus(400).end();
Expand Down
6 changes: 4 additions & 2 deletions server/models/invite.js
@@ -1,3 +1,5 @@
const { escape } = require("sqlstring-sqlite");

const Invite = {
tablename: "invites",
writable: [],
Expand Down Expand Up @@ -69,7 +71,7 @@ const Invite = {
return { invite, error: null };
},
deactivate: async function (inviteId = null) {
const invite = await this.get(`id = ${inviteId}`);
const invite = await this.get(`id = ${escape(inviteId)}`);
if (!invite) return { success: false, error: "Invite does not exist." };
if (invite.status !== "pending")
return { success: false, error: "Invite is not in pending status." };
Expand All @@ -96,7 +98,7 @@ const Invite = {
return { success: true, error: null };
},
markClaimed: async function (inviteId = null, user) {
const invite = await this.get(`id = ${inviteId}`);
const invite = await this.get(`id = ${escape(inviteId)}`);
if (!invite) return { success: false, error: "Invite does not exist." };
if (invite.status !== "pending")
return { success: false, error: "Invite is not in pending status." };
Expand Down
6 changes: 4 additions & 2 deletions server/models/user.js
@@ -1,3 +1,5 @@
const { escape } = require("sqlstring-sqlite");

const User = {
tablename: "users",
writable: [],
Expand Down Expand Up @@ -66,13 +68,13 @@ const User = {
return { user, error: null };
},
update: async function (userId, updates = {}) {
const user = await this.get(`id = ${userId}`);
const user = await this.get(`id = ${escape(userId)}`);
if (!user) return { success: false, error: "User does not exist." };
const { username, password, role, suspended = 0 } = updates;
const toUpdate = { suspended };

if (user.username !== username && username?.length > 0) {
const usedUsername = !!(await this.get(`username = '${username}'`));
const usedUsername = !!(await this.get(`username = ${escape(username)}`));
if (usedUsername)
return { success: false, error: `${username} is already in use.` };
toUpdate.username = username;
Expand Down
3 changes: 2 additions & 1 deletion server/models/workspace.js
Expand Up @@ -2,6 +2,7 @@ const slugify = require("slugify");
const { Document } = require("./documents");
const { checkForMigrations } = require("../utils/database");
const { WorkspaceUser } = require("./workspaceUsers");
const { escape } = require("sqlstring-sqlite");

const Workspace = {
tablename: "workspaces",
Expand Down Expand Up @@ -81,7 +82,7 @@ const Workspace = {
if (!name) return { result: null, message: "name cannot be null" };
var slug = slugify(name, { lower: true });

const existingBySlug = await this.get(`slug = '${slug}'`);
const existingBySlug = await this.get(`slug = ${escape(slug)}`);
if (existingBySlug !== null) {
const slugSeed = Math.floor(10000000 + Math.random() * 90000000);
slug = slugify(`${name}-${slugSeed}`, { lower: true });
Expand Down
3 changes: 2 additions & 1 deletion server/package.json
Expand Up @@ -42,6 +42,7 @@
"slugify": "^1.6.6",
"sqlite": "^4.2.1",
"sqlite3": "^5.1.6",
"sqlstring-sqlite": "^0.1.1",
"swagger-autogen": "^2.23.5",
"swagger-ui-express": "^5.0.0",
"uuid": "^9.0.0",
Expand All @@ -53,4 +54,4 @@
"nodemon": "^2.0.22",
"prettier": "^2.4.1"
}
}
}
4 changes: 2 additions & 2 deletions server/utils/middleware/validApiKey.js
@@ -1,3 +1,4 @@
const { escape } = require("sqlstring-sqlite");
const { ApiKey } = require("../../models/apiKeys");
const { SystemSettings } = require("../../models/systemSettings");

Expand All @@ -14,8 +15,7 @@ async function validApiKey(request, response, next) {
return;
}

const apiKey = await ApiKey.get(`secret = '${bearerKey}'`);
if (!apiKey) {
if (!(await ApiKey.get(`secret = ${escape(bearerKey)}`))) {
response.status(403).json({
error: "No valid api key found.",
});
Expand Down
5 changes: 5 additions & 0 deletions server/yarn.lock
Expand Up @@ -2385,6 +2385,11 @@ sqlite@^4.2.1:
resolved "https://registry.yarnpkg.com/sqlite/-/sqlite-4.2.1.tgz#d4eedfd1ad702f79110792375f4241a90c75c828"
integrity sha512-Tll0Ndvnwkuv5Hn6WIbh26rZiYQORuH1t5m/or9LUpSmDmmyFG89G9fKrSeugMPxwmEIXoVxqTun4LbizTs4uw==

sqlstring-sqlite@^0.1.1:
version "0.1.1"
resolved "https://registry.yarnpkg.com/sqlstring-sqlite/-/sqlstring-sqlite-0.1.1.tgz#c8c61810663f2e59a6b0d737b70a8752bda3a078"
integrity sha512-9CAYUJ0lEUPYJrswqiqdINNSfq3jqWo/bFJ7tufdoNeSK0Fy+d1kFTxjqO9PIqza0Kri+ZtYMfPVf1aZaFOvrQ==

ssri@^8.0.0, ssri@^8.0.1:
version "8.0.1"
resolved "https://registry.yarnpkg.com/ssri/-/ssri-8.0.1.tgz#638e4e439e2ffbd2cd289776d5ca457c4f51a2af"
Expand Down

0 comments on commit dc3dfbf

Please sign in to comment.