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.

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

hookPath and secretToken DoS attacks #1708

Closed
Teselka opened this issue Sep 30, 2022 · 0 comments
Closed

hookPath and secretToken DoS attacks #1708

Teselka opened this issue Sep 30, 2022 · 0 comments
Labels

Comments

@Teselka
Copy link

Teselka commented Sep 30, 2022

Is your feature request related to a problem? Please describe.
So, i've looked on the dependencies list and found module safe-compare that used only in one place (here)
It prevents timing attacks as well but it opens DoS attacks using hookPath and secretToken variables.
Attacker can send requests with very long strings and it will cause DoS because safe compare tries to achieve same response time by iterating over all string characters all the time and with very big string it will take a lot of time.

I've made some tests to prove it

Tests
const safeCompare = require('safe-compare');

const TIMINGATTACK_STRING = 'just a long string to test'.repeat(1000);
const TARGET_STRING = 'my secret token';
const MAX_COMPARISIONS = 1000;

let startTime = performance.now();
let counter = 0;

for (let i=0; i<MAX_COMPARISIONS; i++)
{
  if (TIMINGATTACK_STRING === TARGET_STRING)
      counter++;
}

console.log(`default equal: ${performance.now()-startTime}`);

startTime = performance.now();

for (let i=0; i<MAX_COMPARISIONS; i++)
{
  if (safeCompare(TIMINGATTACK_STRING, TARGET_STRING))
      counter++;
}

console.log(`safe equal: ${performance.now()-startTime}`);

Result:

default equal: 0.05450001358985901
safe equal: 53.27129998803139

Describe the solution you'd like
Use custom function to compare "dangerous" strings. I've made one that will prevent timing attacks on characters only but exposes string length

Custom safeCompare
/**
* Safely compare two strings to prevent timing attacks
* @param a Target string to compare
* @param b Expected string
*/
export function safeCompare(
a: string,
b: string
) {
let result = 0;
if (a.length != b.length) {
  if (b.length < a.length)
    a = b;
  result = 1;
}

for (let i=0; i < a.length; i++) {
  result |= (a.charCodeAt(i) ^ b.charCodeAt(i));
}

return result == 0;
}

Test results:

default equal: 0.05120000243186951
safe equal: 1.2739999890327454
@Teselka Teselka changed the title Hook path and token DoS attacks hookPath and secretToken DoS attacks Sep 30, 2022
@wojpawlik wojpawlik added the bug label Sep 30, 2022
wojpawlik added a commit that referenced this issue Sep 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants