From 1a06e8576c0c55378a8ec856bf326c5c858d7def Mon Sep 17 00:00:00 2001 From: CombatPug Date: Fri, 25 Oct 2024 19:11:11 +0100 Subject: [PATCH 1/2] add better security for limit/offset query params --- src/Data/AccountDataProvider.ts | 7 +++++-- src/dbstore/accounts.ts | 12 ++++++++++++ src/dbstore/cycles.ts | 4 ++++ src/dbstore/originalTxsData.ts | 8 ++++++++ src/dbstore/receipts.ts | 12 ++++++++++++ src/dbstore/transactions.ts | 12 ++++++++++++ 6 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/Data/AccountDataProvider.ts b/src/Data/AccountDataProvider.ts index e767970c..6cde8150 100644 --- a/src/Data/AccountDataProvider.ts +++ b/src/Data/AccountDataProvider.ts @@ -198,9 +198,11 @@ export const provideAccountDataRequest = async ( // AND accountId <= "${accountEnd}" AND accountId >= "${accountStart}" // ORDER BY timestamp, accountId LIMIT ${maxRecords}` + const safeSkip = Number.isInteger(offset) ? offset : 0 + const safeLimit = Number.isInteger(maxRecords) ? maxRecords : 100 const sqlPrefix = `SELECT * FROM accounts WHERE ` - const queryString = `accountId BETWEEN ? AND ? AND timestamp BETWEEN ? AND ? ORDER BY timestamp ASC, accountId ASC LIMIT ${maxRecords}` - const offsetCondition = ` OFFSET ${offset}` + const queryString = `accountId BETWEEN ? AND ? AND timestamp BETWEEN ? AND ? ORDER BY timestamp ASC, accountId ASC LIMIT ${safeLimit}` + const offsetCondition = ` OFFSET ${safeSkip}` let sql = sqlPrefix let values = [] if (accountOffset) { @@ -263,6 +265,7 @@ export const provideAccountDataByListRequest = async ( ): Promise => { const { accountIds } = payload const wrappedAccounts: WrappedStateArray = [] + // todo: does this even work right now? const sql = `SELECT * FROM accounts WHERE accountId IN (?)` const accounts = await Account.fetchAccountsBySqlQuery(sql, accountIds) for (const account of accounts) { diff --git a/src/dbstore/accounts.ts b/src/dbstore/accounts.ts index 5102d603..8607384e 100644 --- a/src/dbstore/accounts.ts +++ b/src/dbstore/accounts.ts @@ -90,6 +90,10 @@ export async function queryAccountByAccountId(accountId: string): Promise { + if (!Number.isInteger(count)) { + Logger.mainLogger.error('Invalid count value') + return null + } try { const sql = `SELECT * FROM accounts ORDER BY cycleNumber DESC, timestamp DESC LIMIT ${ count ? count : 100 @@ -114,6 +118,10 @@ export async function queryLatestAccounts(count: number): Promise { let dbAccounts: DbAccountCopy[] const accounts: AccountsCopy[] = [] + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit value') + return accounts + } try { const sql = `SELECT * FROM accounts ORDER BY cycleNumber ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` dbAccounts = (await db.all(accountDatabase, sql)) as DbAccountCopy[] @@ -174,6 +182,10 @@ export async function queryAccountsBetweenCycles( ): Promise { let dbAccounts: DbAccountCopy[] const accounts: AccountsCopy[] = [] + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit value') + return accounts + } try { const sql = `SELECT * FROM accounts WHERE cycleNumber BETWEEN ? AND ? ORDER BY cycleNumber ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` dbAccounts = (await db.all(accountDatabase, sql, [startCycleNumber, endCycleNumber])) as DbAccountCopy[] diff --git a/src/dbstore/cycles.ts b/src/dbstore/cycles.ts index 80bcdf26..0ab967ec 100644 --- a/src/dbstore/cycles.ts +++ b/src/dbstore/cycles.ts @@ -81,6 +81,10 @@ export async function queryCycleByMarker(marker: string): Promise { } export async function queryLatestCycleRecords(count: number): Promise { + if (!Number.isInteger(count)) { + Logger.mainLogger.error('Invalid count value') + return [] + } try { const sql = `SELECT * FROM cycles ORDER BY counter DESC LIMIT ${count ? count : 100}` const dbCycles = (await db.all(cycleDatabase, sql)) as DbCycle[] diff --git a/src/dbstore/originalTxsData.ts b/src/dbstore/originalTxsData.ts index c0dd1df4..f49c43bf 100644 --- a/src/dbstore/originalTxsData.ts +++ b/src/dbstore/originalTxsData.ts @@ -90,6 +90,10 @@ export async function queryOriginalTxsData( endCycle?: number ): Promise { let originalTxsData: DbOriginalTxData[] = [] + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit') + return originalTxsData + } try { let sql = `SELECT * FROM originalTxsData` const sqlSuffix = ` ORDER BY cycle ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` @@ -166,6 +170,10 @@ export async function queryOriginalTxDataCountByCycles( } export async function queryLatestOriginalTxs(count: number): Promise { + if (!Number.isInteger(count)) { + Logger.mainLogger.error('Invalid count value') + return null + } try { const sql = `SELECT * FROM originalTxsData ORDER BY cycle DESC, timestamp DESC LIMIT ${ count ? count : 100 diff --git a/src/dbstore/receipts.ts b/src/dbstore/receipts.ts index eb54d516..554aed6a 100644 --- a/src/dbstore/receipts.ts +++ b/src/dbstore/receipts.ts @@ -159,6 +159,10 @@ export async function queryReceiptByReceiptId(receiptId: string, timestamp = 0): } export async function queryLatestReceipts(count: number): Promise { + if (!Number.isInteger(count)) { + Logger.mainLogger.error('Invalid count value') + return null + } try { const sql = `SELECT * FROM receipts ORDER BY cycle DESC, timestamp DESC LIMIT ${count ? count : 100}` const receipts = (await db.all(receiptDatabase, sql)) as DbReceipt[] @@ -179,6 +183,10 @@ export async function queryLatestReceipts(count: number): Promise { export async function queryReceipts(skip = 0, limit = 10000): Promise { let receipts: Receipt[] = [] + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit') + return receipts + } try { const sql = `SELECT * FROM receipts ORDER BY cycle ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` receipts = (await db.all(receiptDatabase, sql)) as DbReceipt[] @@ -261,6 +269,10 @@ export async function queryReceiptsBetweenCycles( endCycleNumber: number ): Promise { let receipts: Receipt[] = [] + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit') + return receipts + } try { const sql = `SELECT * FROM receipts WHERE cycle BETWEEN ? AND ? ORDER BY cycle ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` receipts = (await db.all(receiptDatabase, sql, [startCycleNumber, endCycleNumber])) as DbReceipt[] diff --git a/src/dbstore/transactions.ts b/src/dbstore/transactions.ts index c0d40a86..916007a7 100644 --- a/src/dbstore/transactions.ts +++ b/src/dbstore/transactions.ts @@ -99,6 +99,10 @@ export async function queryTransactionByAccountId(accountId: string): Promise { + if (!Number.isInteger(count)) { + Logger.mainLogger.error('Invalid count value') + return null + } try { const sql = `SELECT * FROM transactions ORDER BY cycleNumber DESC, timestamp DESC LIMIT ${ count ? count : 100 @@ -123,6 +127,10 @@ export async function queryLatestTransactions(count: number): Promise { let transactions + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit') + return null + } try { const sql = `SELECT * FROM transactions ORDER BY cycleNumber ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` transactions = (await db.all(transactionDatabase, sql)) as DbTransaction[] // TODO: confirm structure of object from db @@ -189,6 +197,10 @@ export async function queryTransactionsBetweenCycles( endCycleNumber: number ): Promise { let transactions + if (!Number.isInteger(skip) || !Number.isInteger(limit)) { + Logger.mainLogger.error('Invalid skip or limit value') + return null + } try { const sql = `SELECT * FROM transactions WHERE cycleNumber BETWEEN ? AND ? ORDER BY cycleNumber ASC, timestamp ASC LIMIT ${limit} OFFSET ${skip}` transactions = (await db.all(transactionDatabase, sql, [ From 207ccadaea5538c475ed58bafb98ba8a94c1e0c6 Mon Sep 17 00:00:00 2001 From: CombatPug Date: Mon, 28 Oct 2024 12:22:51 +0000 Subject: [PATCH 2/2] fix: prefix logs with method name --- src/dbstore/accounts.ts | 6 +++--- src/dbstore/cycles.ts | 2 +- src/dbstore/originalTxsData.ts | 4 ++-- src/dbstore/receipts.ts | 6 +++--- src/dbstore/transactions.ts | 6 +++--- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/dbstore/accounts.ts b/src/dbstore/accounts.ts index 8607384e..6b2455aa 100644 --- a/src/dbstore/accounts.ts +++ b/src/dbstore/accounts.ts @@ -91,7 +91,7 @@ export async function queryAccountByAccountId(accountId: string): Promise { if (!Number.isInteger(count)) { - Logger.mainLogger.error('Invalid count value') + Logger.mainLogger.error('queryLatestAccounts - Invalid count value') return null } try { @@ -119,7 +119,7 @@ export async function queryAccounts(skip = 0, limit = 10000): Promise { export async function queryLatestCycleRecords(count: number): Promise { if (!Number.isInteger(count)) { - Logger.mainLogger.error('Invalid count value') + Logger.mainLogger.error('queryLatestCycleRecords - Invalid count value') return [] } try { diff --git a/src/dbstore/originalTxsData.ts b/src/dbstore/originalTxsData.ts index f49c43bf..4ab8029b 100644 --- a/src/dbstore/originalTxsData.ts +++ b/src/dbstore/originalTxsData.ts @@ -91,7 +91,7 @@ export async function queryOriginalTxsData( ): Promise { let originalTxsData: DbOriginalTxData[] = [] if (!Number.isInteger(skip) || !Number.isInteger(limit)) { - Logger.mainLogger.error('Invalid skip or limit') + Logger.mainLogger.error('queryOriginalTxsData - Invalid skip or limit') return originalTxsData } try { @@ -171,7 +171,7 @@ export async function queryOriginalTxDataCountByCycles( export async function queryLatestOriginalTxs(count: number): Promise { if (!Number.isInteger(count)) { - Logger.mainLogger.error('Invalid count value') + Logger.mainLogger.error('queryLatestOriginalTxs - Invalid count value') return null } try { diff --git a/src/dbstore/receipts.ts b/src/dbstore/receipts.ts index 554aed6a..009a6744 100644 --- a/src/dbstore/receipts.ts +++ b/src/dbstore/receipts.ts @@ -160,7 +160,7 @@ export async function queryReceiptByReceiptId(receiptId: string, timestamp = 0): export async function queryLatestReceipts(count: number): Promise { if (!Number.isInteger(count)) { - Logger.mainLogger.error('Invalid count value') + Logger.mainLogger.error('queryLatestReceipts - Invalid count value') return null } try { @@ -184,7 +184,7 @@ export async function queryLatestReceipts(count: number): Promise { export async function queryReceipts(skip = 0, limit = 10000): Promise { let receipts: Receipt[] = [] if (!Number.isInteger(skip) || !Number.isInteger(limit)) { - Logger.mainLogger.error('Invalid skip or limit') + Logger.mainLogger.error('queryReceipts - Invalid skip or limit') return receipts } try { @@ -270,7 +270,7 @@ export async function queryReceiptsBetweenCycles( ): Promise { let receipts: Receipt[] = [] if (!Number.isInteger(skip) || !Number.isInteger(limit)) { - Logger.mainLogger.error('Invalid skip or limit') + Logger.mainLogger.error('queryReceiptsBetweenCycles - Invalid skip or limit') return receipts } try { diff --git a/src/dbstore/transactions.ts b/src/dbstore/transactions.ts index 916007a7..47fc8d2c 100644 --- a/src/dbstore/transactions.ts +++ b/src/dbstore/transactions.ts @@ -100,7 +100,7 @@ export async function queryTransactionByAccountId(accountId: string): Promise { if (!Number.isInteger(count)) { - Logger.mainLogger.error('Invalid count value') + Logger.mainLogger.error('queryLatestTransactions - Invalid count value') return null } try { @@ -128,7 +128,7 @@ export async function queryLatestTransactions(count: number): Promise { let transactions if (!Number.isInteger(skip) || !Number.isInteger(limit)) { - Logger.mainLogger.error('Invalid skip or limit') + Logger.mainLogger.error('queryTransactions - Invalid skip or limit') return null } try { @@ -198,7 +198,7 @@ export async function queryTransactionsBetweenCycles( ): Promise { let transactions if (!Number.isInteger(skip) || !Number.isInteger(limit)) { - Logger.mainLogger.error('Invalid skip or limit value') + Logger.mainLogger.error('queryTransactionsBetweenCycles - Invalid skip or limit value') return null } try {