jonschlinkert / gray-matter

Smarter YAML front matter parser, used by metalsmith, Gatsby, Netlify, Assemble, mapbox-gl, phenomic, vuejs vitepress, TinaCMS, Shopify Polaris, Ant Design, Astro, hashicorp, garden, slidev, saber, sourcegraph, and many others. Simple to use, and battle tested. Parses YAML by default but can also parse JSON Front Matter, Coffee Front Matter, TOML Front Matter, and has support for custom parsers. Please follow gray-matter's author: https://github.com/jonschlinkert
https://github.com/jonschlinkert
MIT License
3.91k stars 138 forks source link

Security consideration: disable JS engine by default to prevent RCEs in dependents #131

Open simonhaenisch opened 3 years ago

simonhaenisch commented 3 years ago

I created a remote code execution (RCE) vulnerability in a package of mine (see https://github.com/simonhaenisch/md-to-pdf/issues/99) because I was somehow not aware that gray-matter was able to parse JS as well, and that it was enabled by default.

A few years back I simply used gray-matter in my tool md-to-pdf to parse YAML front-matter without going through the whole docs (initially this was a personal project for writing assignments). My package ended up being used on dillinger.io (web-based markdown editor) for PDF exports, and thus you could execute code on their server by adding a front-matter that started with ---js (i. e., dynamic language detection would kick in). To fix it, I overwrote the js engine in the gray-matter options, and users can provide custom gray-matter options if they want it back.

BTW this has also been somewhat discussed in #112 but the goal of this ticket is different. I'm suggesting that the JS engine is disabled by default, and only works if you explicitly set language: 'js' in the options. I know this will be a breaking change but I can imagine there would be other packages out there that run on servers and use gray-matter to parse user-provided content, and not all of them might have been aware of this JS parsing feature either.

IMO the responsibility is usually on the package user because you should never trust user-provided content, however in this case it's easy to prevent developer mistakes by making this an actual opt-in option (instead of already opted-in by default).

segevfiner commented 1 year ago

This should really not be enabled by default...