search

wilk / microjob

A tiny wrapper for turning Node.js worker threads into easy-to-use routines for heavy CPU loads.
https://wilk.github.io/microjob/
MIT License
2.02k stars 47 forks source link

Issue #2: Sanitize #60

Closed tomjw64 closed 4 years ago

tomjw64 commented 4 years ago

I noticed a part of the guide that mentioned a security issue stemming from string input sanitizing for the ctx argument passed to job. Link: #2

I was surprised to see that when assigning the string context variable that the only thing that seemed to go on was wrapping the string in a set of single-quotes:

case 'string':
  variable = `'${config.ctx[key]}'`
  break

which allows the exploit shown in #2 to occur. This PR simply uses JSON.stringify instead, just like it is already done with object types to stop someone from doing some quote-related funny-business. I won't claim that this makes everything particularly safe, but it does solve the simple case shown in the issue description. Perhaps the docs can be updated to show how such an exploit could still occur.

This PR also adds a test that checks, and fails on master, but passes on this PR, for allowance of arbitrary code execution in strings passed to the ctx argument of job.

wilk commented 4 years ago

Makes sense! Sorry for the delay and thanks for contributing!