zxcvbn-ts / zxcvbn

Low-Budget Password Strength Estimation
https://www.usenix.org/conference/usenixsecurity16/technical-sessions/presentation/wheeler
MIT License
907 stars 72 forks source link

JavaScript heap out of memory #74

Closed netanel-haber closed 3 years ago

netanel-haber commented 3 years ago

Importing the common and en language packages using webpack (under CRA4) causes the dev server memory usage to skyrocket upon start, resulting in an immediate V8 out-of-memory crash.

Usage:

import zxcvbnCommonPackage from '@zxcvbn-ts/language-common/'
import zxcvbnEnPackage from '@zxcvbn-ts/language-en/dist/'

const password = 'somePassword'
const options = {
  translations: zxcvbnEnPackage.translations,
  graphs: zxcvbnCommonPackage.adjacencyGraphs,
  dictionary: {
    ...zxcvbnCommonPackage.dictionary,
    ...zxcvbnEnPackage.dictionary,
  },
}

ZxcvbnOptions.setOptions(options)

zxcvbn(password)

Stack trace:


FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
 1: 00007FF60F0C412F napi_wrap+133311
 2: 00007FF60F05DD06 SSL_get_quiet_shutdown+63062
 3: 00007FF60F05EB9D node::OnFatalError+301
 4: 00007FF60F9419CE v8::Isolate::ReportExternalAllocationLimitReached+94     
 5: 00007FF60F9267BD v8::SharedArrayBuffer::Externalize+781
 6: 00007FF60F7CFFCC v8::internal::Heap::EphemeronKeyWriteBarrierFromCode+1516
 7: 00007FF60F7DB3EA v8::internal::Heap::ProtectUnprotectedMemoryChunks+1258  
 8: 00007FF60F7D8529 v8::internal::Heap::PageFlagsAreConsistent+2457
 9: 00007FF60F7CD0C1 v8::internal::Heap::CollectGarbage+2049
10: 00007FF60F7CB2C5 v8::internal::Heap::AllocateExternalBackingStore+1349    
11: 00007FF60F7EB73B v8::internal::Factory::NewFillerObject+203
12: 00007FF60F519E0F v8::internal::interpreter::JumpTableTargetOffsets::iterator::operator=+1039
13: 00007FF60F9CCDFD v8::internal::SetupIsolateDelegate::SetupHeap+474253
14: 0000011BD541E117
npm ERR! code ELIFECYCLE
npm ERR! errno 134
npm ERR! my-app@0.1.0 start: `react-scripts start`
npm ERR! Exit status 134
npm ERR! 
npm ERR! Failed at the my-app@0.1.0 start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.```
MrWook commented 3 years ago

You need to be aware that the dictionaries are huge there are 2MB of plain old json data.

I can say for myself that i never encountered this issue but i am using vue and nuxt instead of react. On the other side im using webpack for vue and nuxt too. Does this happen for a new project or do you included it into a bigger project?

netanel-haber commented 3 years ago

I've tested it in multiple environments including entirely new projects. I've also used the node max_old_space_size env with as much as 8GB of ram (It should be noted that it isn't the browser tab that crashes, it's the actual dev process).

The problem isn't some absolute minimum memory footprint - but that like I've mentioned, regardless of the initial max memory constraint, the usage just immediately skyrockets and keeps climbing until hitting the max and crashing the process. This seems to actually stem from a real memory leak that has something to do with webpack-dev-server interop, as running the module in a simple node project (not CRA, just plain old node), causes no problems.

It should also be noted that this hasn't happened to me with the original zxcvbn package, just with zxcvbn-ts.

MrWook commented 3 years ago

This is weird, there are no functions that could go into a memory leak but i will check it out 🤔

Wich version are you using?

netanel-haber commented 3 years ago

I'm using the latest version. Another thing to add is that both JS and TS templates of CRA crashed for me, on different computers as well.

MrWook commented 3 years ago

I don't think it is a direct issue with zxcvbn-ts because it will already crash if you only import the password.js file into a fresh repo. The content of this file is only one variable with a big array.

But it seems like the approach from the original repo is more fit for big data like that. In the original repo those dictionaries are not an array but a string that is splitted on runtime. I thought it would be a good idea to directly use an array to increase runtime performance but it seems like this comes with a downside for some dev servers 🤔

I will adjust the dictionaries so that they match the original initalization with the split function. Thanks for the finding 👍

netanel-haber commented 3 years ago

Interesting! Thank you.

MrWook commented 3 years ago

I just release version 1.0.0 with this issue included. In my starter setup for CRA4 it worked now. Can you try it out for youself?

netanel-haber commented 3 years ago

It seems to work now - thank you very much.