jq icon indicating copy to clipboard operation
jq copied to clipboard

Security considerations

Open lmeyerov opened this issue 7 years ago • 10 comments

We're considering allowing user-defined transforms specified in jq, but it has been difficult for us to ascertain how safe it is from code injection & privilege escalation, by design & in practice. As far as I can tell, while jq supports some user-defined functions, they are largely sandboxed. The exception is the module imports, which would need some thing for restricting.

Can anyone clarify the security model? May help to link the answer as part of the README.

lmeyerov avatar Mar 05 '17 03:03 lmeyerov

Excellent question. So far we've held back from adding the sorts of builtins that might make it easier to break out of the jq sandbox.

Real or potential issues to be concerned with:

  • CPU consumption -- jq is Turing complete (but single-threaded)
  • Row-hammer attacks -- there's no reason to think that they are not possible to code in jq
  • Potential security bugs in jq -- we've not yet applied AFL, but others have and we know that there exist JSON parser bugs
  • Potential security bugs in bison where the jq program could source itself can exploit them
  • It is possible to read arbitrary files down from the current directory using include and import directives -- this is a big deal if you don't trust the jq code you're running
  • We do want to add more I/O builtins in the future -- you can't count on jq being sandboxed forever

Now, it is my intention to require the use of a command-line switch to enable future I/O builtins that could allow reading or writing files, or executing external programs. But still, there's no way to guarantee that's so.

I think it's probably desirable to add a command-line option to disallow the main program from including files or modules, or more generally a sandbox option.

nicowilliams avatar Mar 05 '17 05:03 nicowilliams

See also #1215, #660, and others.

nicowilliams avatar Mar 05 '17 05:03 nicowilliams

Ah, excellent. Thanks!

Out of those, the include and import statements seem concerning under most threat models (including ours.) +1 for sandboxed mode flag for this and future features :) Items like DOS via resource abuse are real concerns, and I agree with others that we'd normally go to to the OS etc. for handling that.

For the interim, would it be sufficient for us to do a blacklist for "include" and "import", and freeze to the current version?

lmeyerov avatar Mar 06 '17 21:03 lmeyerov

@lmeyerov Yes. You might even edit src/parser.y and remove the include/import functionality.

Thinking about how to disable include/import... we could just shove that into struct locfile, or add another parse param for this, and the in the rules for include/import simply error out if they are disabled.

nicowilliams avatar Mar 07 '17 05:03 nicowilliams

There's a lot of potential sandboxing options:

  • allow importing modules, but not data
  • allow importing modules, but not data, and don't allow modules to import data either
  • don't allow importing anything

Eventually we'll have fairly capable I/O builtins. We might have a need for many sandboxing options.

Obviously, @lmeyerov, the simplest sandbox will be to chroot jq...

nicowilliams avatar Mar 07 '17 05:03 nicowilliams

Actually, that may well be the best thing we could do: have a "chroot" option that disallows I/O to/from files outside the sandbox. On Unix-like systems jq would then literally call chroot(2) and be done, but on Windows it might attempt to implement a chroot-like sandbox for the jq program in user-land.

nicowilliams avatar Mar 07 '17 05:03 nicowilliams

It may help, as simple policies, to consider two of the most basic security properties: data confidentiality & integrity. I'd expect "--sandbox" to provide both. Integrity doesn't seem to be an issue, but confidentiality would seem to require disallowing both module & data import, as who knows what's on the system. (I'm assuming the attacker can control the data & transform parameters, but not the rest, and there is a chance of dangerous files already on the system for use as part of an escalation.)

Making "--sandbox" target a model like that, and supporting it going forwards, would provide a simple policy for both use & development.

FWIW, we're using JQ via https://github.com/sanack/node-jq, so patching it with --sandbox would be easy. Rather not have them fork which jq..

lmeyerov avatar Mar 12 '17 04:03 lmeyerov

This thread is now almost 4 years old, but seems like it should be raised again.

We are very much interested in a "--sandbox" option. Our use case is that a client can supply a JQ filter allow with an RestAPI call so that the server only returns a subset of the data. Note: GraphQL and similar are not a good fit for us at this time.

I am loath to re-implement the syntax, create a library to sanitize the input (which would be just as difficult as reimplementing jq), or use a custom built version which does not contain the insecure builtins and variables. Is there anything we can do to contribute to a "--sandbox" option or an alternative build with those insecurities taken out, suitable for embedding?

raijinsetsu avatar Jan 20 '22 17:01 raijinsetsu