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

Shard 1426 #128

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Shard 1426 #128

wants to merge 6 commits into from
+1,523 −618

Conversation

devendra-shardeum
Copy link
Contributor

Added prepared statements everywhere in archiver
now it's faster and secured against sql injections

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 15, 2025
View reviewed changes
src/dbstore/accounts.ts
if (config.VERBOSE) {
Logger.mainLogger.debug('Account accounts', accounts ? accounts.length : accounts, 'skip', skip)
Logger.mainLogger.debug('Account accounts', accounts.length, 'skip', skip, 'limit', limit);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 4 hours ago

To fix the problem, we need to sanitize the skip parameter before logging it. This can be done by removing any newline characters from the skip parameter using String.prototype.replace. This ensures that the log entry cannot be manipulated by injecting special characters.

  • In the file src/dbstore/accounts.ts, sanitize the skip parameter before logging it.
  • Add a utility function to sanitize user input by removing newline characters.
  • Use this utility function to sanitize the skip parameter before logging it.
Suggested changeset 1
src/dbstore/accounts.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/accounts.ts b/src/dbstore/accounts.ts
--- a/src/dbstore/accounts.ts
+++ b/src/dbstore/accounts.ts
@@ -233,3 +233,4 @@
   if (config.VERBOSE) {
-    Logger.mainLogger.debug('Account accounts', accounts.length, 'skip', skip, 'limit', limit);
+    const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
+    Logger.mainLogger.debug('Account accounts', accounts.length, 'skip', sanitizedSkip, 'limit', limit);
   }
EOF
@@ -233,3 +233,4 @@
if (config.VERBOSE) {
Logger.mainLogger.debug('Account accounts', accounts.length, 'skip', skip, 'limit', limit);
const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
Logger.mainLogger.debug('Account accounts', accounts.length, 'skip', sanitizedSkip, 'limit', limit);
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/dbstore/cycles.ts
if (config.VERBOSE) {
Logger.mainLogger.debug('Updated cycle for counter', cycle.cycleRecord.counter, cycle.cycleMarker)
Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 3 hours ago

To fix the log injection issue, we need to sanitize the cycle.counter value before logging it. Specifically, we should remove any newline characters from the cycle.counter value to prevent log injection attacks. This can be done using the String.prototype.replace method.

Suggested changeset 1
src/dbstore/cycles.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/cycles.ts b/src/dbstore/cycles.ts
--- a/src/dbstore/cycles.ts
+++ b/src/dbstore/cycles.ts
@@ -90,3 +90,4 @@
     if (config.VERBOSE) {
-      Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);
+      const sanitizedCounter = String(cycle.counter).replace(/\n|\r/g, "");
+      Logger.mainLogger.debug('Updated cycle for counter', sanitizedCounter, marker);
     }
EOF
@@ -90,3 +90,4 @@
if (config.VERBOSE) {
Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);
const sanitizedCounter = String(cycle.counter).replace(/\n|\r/g, "");
Logger.mainLogger.debug('Updated cycle for counter', sanitizedCounter, marker);
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/dbstore/cycles.ts
if (config.VERBOSE) {
Logger.mainLogger.debug('Updated cycle for counter', cycle.cycleRecord.counter, cycle.cycleMarker)
Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 3 hours ago

To fix the log injection issue, we need to sanitize the marker variable before logging it. This can be done by removing any newline characters from the marker string using String.prototype.replace. This ensures that user input cannot inject new log entries.

Suggested changeset 1
src/dbstore/cycles.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/cycles.ts b/src/dbstore/cycles.ts
--- a/src/dbstore/cycles.ts
+++ b/src/dbstore/cycles.ts
@@ -90,3 +90,4 @@
     if (config.VERBOSE) {
-      Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);
+      const sanitizedMarker = marker.replace(/\n|\r/g, "");
+      Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, sanitizedMarker);
     }
EOF
@@ -90,3 +90,4 @@
if (config.VERBOSE) {
Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, marker);
const sanitizedMarker = marker.replace(/\n|\r/g, "");
Logger.mainLogger.debug('Updated cycle for counter', cycle.counter, sanitizedMarker);
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/dbstore/cycles.ts
Logger.mainLogger.error(e)
Logger.mainLogger.error('Unable to update Cycle', cycle.cycleMarker)
Logger.mainLogger.error(e);
Logger.mainLogger.error('Unable to update Cycle', marker);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 2 hours ago

To fix the log injection issue, we need to sanitize the marker variable before logging it. This can be done by removing any newline characters from the marker string. We will use the String.prototype.replace method to achieve this. The changes will be made in the updateCycle function in the src/dbstore/cycles.ts file.

Suggested changeset 1
src/dbstore/cycles.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/cycles.ts b/src/dbstore/cycles.ts
--- a/src/dbstore/cycles.ts
+++ b/src/dbstore/cycles.ts
@@ -94,3 +94,4 @@
     Logger.mainLogger.error(e);
-    Logger.mainLogger.error('Unable to update Cycle', marker);
+    const sanitizedMarker = marker.replace(/\n|\r/g, "");
+    Logger.mainLogger.error('Unable to update Cycle', sanitizedMarker);
   }
EOF
@@ -94,3 +94,4 @@
Logger.mainLogger.error(e);
Logger.mainLogger.error('Unable to update Cycle', marker);
const sanitizedMarker = marker.replace(/\n|\r/g, "");
Logger.mainLogger.error('Unable to update Cycle', sanitizedMarker);
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/dbstore/receipts.ts
}

if (config.VERBOSE) {
Logger.mainLogger.debug('Receipt receipts', receipts.length, 'skip', skip);

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 3 hours ago

To fix the problem, we need to sanitize the skip parameter before logging it. This can be done by converting the skip parameter to a string and removing any newline characters. This ensures that the log entry cannot be manipulated by injecting special characters.

  • In the queryReceipts function, sanitize the skip parameter before logging it.
  • Use String.prototype.replace to remove any newline characters from the skip parameter.
  • Ensure that the sanitized skip parameter is used in the log statement.
Suggested changeset 1
src/dbstore/receipts.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/receipts.ts b/src/dbstore/receipts.ts
--- a/src/dbstore/receipts.ts
+++ b/src/dbstore/receipts.ts
@@ -267,3 +267,4 @@
     if (config.VERBOSE) {
-      Logger.mainLogger.debug('Receipt receipts', receipts.length, 'skip', skip);
+      const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
+      Logger.mainLogger.debug('Receipt receipts', receipts.length, 'skip', sanitizedSkip);
     }
EOF
@@ -267,3 +267,4 @@
if (config.VERBOSE) {
Logger.mainLogger.debug('Receipt receipts', receipts.length, 'skip', skip);
const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
Logger.mainLogger.debug('Receipt receipts', receipts.length, 'skip', sanitizedSkip);
}
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
src/dbstore/transactions.ts
'Transaction transactions',
transactions ? transactions.length : transactions,
'skip',
skip

Check warning

Code scanning / CodeQL

Log injection Medium

Log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI about 3 hours ago

To fix the log injection issue, we need to sanitize the skip parameter before logging it. Specifically, we should remove any newline characters from the skip parameter to prevent log injection. This can be done using String.prototype.replace to ensure no line endings are present in the user input.

Suggested changeset 1
src/dbstore/transactions.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/dbstore/transactions.ts b/src/dbstore/transactions.ts
--- a/src/dbstore/transactions.ts
+++ b/src/dbstore/transactions.ts
@@ -202,2 +202,3 @@
     if (config.VERBOSE) {
+      const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
       Logger.mainLogger.debug(
@@ -206,3 +207,3 @@
         'skip',
-        skip
+        sanitizedSkip
       );
EOF
@@ -202,2 +202,3 @@
if (config.VERBOSE) {
const sanitizedSkip = String(skip).replace(/\n|\r/g, "");
Logger.mainLogger.debug(
@@ -206,3 +207,3 @@
'skip',
skip
sanitizedSkip
);
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aniketdivekar aniketdivekar Awaiting requested review from aniketdivekar

@mssabr01 mssabr01 Awaiting requested review from mssabr01

@mgthuramoemyint mgthuramoemyint Awaiting requested review from mgthuramoemyint

@S0naliThakur S0naliThakur Awaiting requested review from S0naliThakur

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@devendra-shardeum