parallax / jsPDF

Client-side JavaScript PDF generation for everyone.
https://parall.ax/products/jspdf
MIT License
29.38k stars 4.68k forks source link

While running OWASP scanner, JsPdf package giving Vulnerable Dependency on btoa@1.2.1 #3278

Closed rashidaslam75 closed 2 years ago

rashidaslam75 commented 3 years ago

Description:

btoa for Node.JS (it's a one-liner)

Detail:

CWE-125: Out-of-bounds Read (OSSINDEX) suppress

The software reads data past the end, or before the beginning, of the intended buffer. CWE-125 Out-of-bounds Read

CVSSv3: Base Score: MEDIUM (6.5) Vector: CVSS:/AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:H

References: OSSINDEX - CWE-125: Out-of-bounds Read Vulnerable Software & Versions (OSSINDEX):

cpe:2.3:a::btoa:1.2.1:::::::

AbstractAlao commented 3 years ago

Also to add on to this Insecure Randomness: jspdf.es-2.4.0.min.js:86 The random number generator implemented by random() cannot withstand a cryptographic attack. Command Injection: The method set() in jspdf.es-2.4.0.min.js calls exec() to execute a command. This call might allow an attacker to inject malicious commands.

Uzlopak commented 3 years ago

Math.random is a false positive. We don't need cryptograhical secure random numbers. It is only used to get some identifiers for the PubSub-System and FileIds. The only thing I can think of is if it is a small chance of collision possible were PubSub breaks... But that is I think neglectable.

HackbrettXXX commented 3 years ago

@rashidaslam75 what would be a fix for this? Please provide a PR.

AbstractAlao commented 3 years ago

@Uzlopak and @HackbrettXXX I think a function to sanitize inputs before passing them into exec() would do it. Example from public static string SanitizeUrl(string url) { return Regex.Replace(url, @"[^-A-Za-z0-9+&@#/%?=~_|!:,.;\(\)]", ""); } Seems the scanner picked up on the following line

image

AbstractAlao commented 3 years ago

Math.random is a false positive. We don't need cryptograhical secure random numbers. It is only used to get some identifiers for the PubSub-System and FileIds. The only thing I can think of is if it is a small chance of collision possible were PubSub breaks... But that is I think neglectable.

I agree this is most likely a false positive since you are not using the function to create a secure signature. I found this StackOverflow for reference.

mshah-aiondigital commented 3 years ago

Can we replace btoa package usage with a simple one liner?

 function btoa(str) {
  return new Buffer(str).toString('base64')
}
rashidaslam75 commented 3 years ago

@HackbrettXXX can we consider @mshah-aiondigital suggestions here - seems this would give us the required results ?

HackbrettXXX commented 3 years ago

@Uzlopak and @HackbrettXXX I think a function to sanitize inputs before passing them into exec() would do it. Example from public static string SanitizeUrl(string url) { return Regex.Replace(url, @"[^-A-Za-z0-9+&@#/%?=~_|!:,.;\(\)]", ""); } Seems the scanner picked up on the following line

image

@AbstractAlao TBH I don't see where the problem is there. Attackers cannot execute code here. The only issue there might be is the regex might run relatively long for long URLs.

@mshah-aiondigital The btoa package does something very similar to that:

  function btoa(str) {
    var buffer;

    if (str instanceof Buffer) {
      buffer = str;
    } else {
      buffer = Buffer.from(str.toString(), 'binary');
    }

    return buffer.toString('base64');
  }

So any security issues this package has, your one liner probably will have the same.

AbstractAlao commented 3 years ago

@HackbrettXXX from what I understand the dataUrlParts are un-sanitized inputs so someone could pass the Githubissues.

  • Githubissues is a development platform for aggregating issues.