votingworks / vxsuite

https://voting.works
30 stars 6 forks source link

VxSuite: limit the size of files when using `readFile` #4443

Open eventualbuddha opened 10 months ago

eventualbuddha commented 10 months ago

Loading arbitrary files with unknown size is a bad idea. It can cause the process to become super slow and exhaust system memory, leading to slow UIs and swapping at best and crashes or unrecoverable freezing at worst. In general, we should not even call readFile most of the time and instead use a stream that reads the file a chunk at a time. There is some work to be done to make that more ergonomic.

In cases where it does make sense to call readFile to get all the file data, I propose that we be required to explicitly specify the maximum amount of data to read as an option to a custom readFile in @votingworks/backend that takes a maxSize option. We should add a custom ESLint rule prohibiting readFile and readFileSync from fs, fs/promises, and fs-extra with an exception for test files, hinting to use readFile from @votingworks/backend with a specific maxSize value.

Because the new readFile would be async, this would effectively prohibit the use of readFileSync in non-test code as a side-effect, which might be a minor inconvenience.

See Slack thread here.

eventualbuddha commented 10 months ago

I did a spike on this here: https://github.com/votingworks/vxsuite/compare/brian/feat/backend/readfile-maxsize