Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BLUE-256 Fix the issue in the isDebugMiddleware #63

Merged
merged 8 commits into from
Sep 2, 2024
Merged

BLUE-256 Fix the issue in the isDebugMiddleware #63

merged 8 commits into from
Sep 2, 2024

Conversation

jairajdev
Copy link
Contributor

No description provided.

@github-actions GitHub Actions
Copy link

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review

Configuration Structure
The new nested ARCHIVER_DATA object in the Config interface introduces multiple database configurations which could lead to confusion or misconfiguration if not properly documented or validated.

Error Handling
The error handling within the isReceiptRobust function could be improved by adding more specific error messages or types, rather than generic 'verification failed' messages, to aid in debugging and maintenance.

Database Initialization
The init function now initializes multiple database connections based on the new ARCHIVER_DATA configuration. This change increases complexity and potential for errors in database handling. It's crucial to ensure that all databases are correctly configured and handled.

github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 14, 2024
View reviewed changes
src/Data/Collector.ts Outdated
`The account verification failed from robustQuery nodes ${receipt.tx.txId} , ${receipt.cycle}, ${receipt.tx.timestamp}`
)
continue
if (config.verifyAccountData) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/Data/Collector.ts Outdated
`The receipt validation failed from robustQuery nodes ${receipt.tx.txId} , ${receipt.cycle}, ${receipt.tx.timestamp}`
)
continue
if (config.verifyReceiptData) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/Data/Collector.ts Fixed Show fixed Hide fixed
src/Data/Collector.ts Fixed Show fixed Hide fixed
src/dbstore/sqlite3storage.ts
}

const createDB = async (dbPath: string, dbName: string): Promise<Database> => {
console.log('dbName', dbName, 'dbPath', dbPath)

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 24, 2024
View reviewed changes
src/Data/Collector.ts Outdated
const existingReceipt = await Receipt.queryReceiptByReceiptId(txId)
const failedReasons = []
const nestedCounterMessages = []
if (config.verifyAppReceiptData) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/Data/Collector.ts Outdated
return { success: false, failedReasons, nestedCounterMessages }
}
}
if (config.verifyAccountData) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/Data/Collector.ts Outdated
return { success: false, failedReasons, nestedCounterMessages }
}
}
if (config.verifyReceiptSignaturesSeparately) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/Data/Collector.ts Outdated
timestamp
)

if (config.verifyReceiptData) {

Check failure

Code scanning / CodeQL

User-controlled bypass of security check High

This condition guards a sensitive
action
Loading
, but a
user-provided value
Loading
controls it.
src/primary-process/index.ts
worker.kill()
}
if (receiptLoadTraker < config.receiptLoadTrakerLimit) {
console.log(`Receipt load is below the limit: ${receiptLoadTraker}/${config.receiptLoadTrakerLimit}`)

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.
src/primary-process/index.ts
}, based on ${receiptLoadTraker} receipts received.`
)
receiptLoadTraker = 0 // Reset the count
}, config.receiptLoadTrakerInterval)

Check failure

Code scanning / CodeQL

Resource exhaustion High

This creates a timer with a user-controlled duration from a
user-provided value
Loading
.
src/worker-process/index.ts Outdated
process.send({ type: 'child_ready' })
setInterval(() => {
console.log(
`lastActivityCheckTimeout: ${config.lastActivityCheckTimeout}, lastActivityCheckInterval: ${config.lastActivityCheckInterval}`

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.
src/worker-process/index.ts Outdated
console.log(`Worker ${process.pid} is idle for more than 1 minute`)
process.send({ type: 'child_close' })
}
}, config.lastActivityCheckInterval)

Check failure

Code scanning / CodeQL

Resource exhaustion High

This creates a timer with a user-controlled duration from a
user-provided value
Loading
.
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 25, 2024
View reviewed changes
src/dbstore/index.ts

function createDirectories(pathname: string): void {
const __dirname = path.resolve()
pathname = pathname.replace(/^\.*\/|\/?[^/]+\.[a-z]+|\/$/g, '') // Remove leading directory markers, and remove ending /file-name.extension

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on
a user-provided value
Loading
may run slow on strings with many repetitions of '.'.
@jairajdev jairajdev changed the title Fix the issue in the isDebugMiddleware BLUE-256 Fix the issue in the isDebugMiddleware Sep 1, 2024
@mhanson-github mhanson-github merged commit 2b3fc53 into dev Sep 2, 2024
5 of 7 checks passed
@mhanson-github mhanson-github deleted the isDebugMode-update branch September 5, 2024 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Review effort [1-5]: 4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jairajdev @mhanson-github