Skip to content

Replacements break with unrelated user provided data #13817

@pedroassis

Description

@pedroassis

Issue Creation Checklist

[x] I have read the contribution guidelines
https://github.com/HandleCollections/sequelize-sscce

Bug Description

When using replacements with user provided data we get errors when users input the colon symbol.

This happens because the user string is injected into the sql which later is parsed by sql-string.js file's formatNamedParameters function.

The formatNamedParameters does not have a way to know where the colon symbols comes from.

SSCCE

Here is the link to the SSCCE for this issue:
https://github.com/HandleCollections/sequelize-sscce

  // lib/sql-string.js formatNamedParameters gets the sql with `WHERE name = "some:foo"`
  // it will try to replace the ":foo", even tho it is not supposed to get replaced
  const query = {
    where: {
      name: 'some:foo', // this is a user provided string I expect to be sanitized
    },
    replacements: {
      test: 'this is used somewhere', // not used in example for the sake of simplicity
    },
  };

What do you expect to happen?

I want to be able to search for strings containing a colon symbol and use replacements at the same time.

I also expect user provided data to be sanitized and never be treated as anything other than text.

What is actually happening?

sequelize tries to parse user provided data and throws an error.

I think it might be an avenue for sql injection since sequelize is treating an user provided string as parseable text.

Error: Named parameter ":foo" has no value in the given object.
    at /Users/pedro/projects/sequelize-sscce/node_modules/sequelize/dist/lib/sql-string.js:103:11

Environment

  • Sequelize version: 6.12.2 and 4.4.4
  • Node.js version: v14.16.0

Bug Report Checklist

How does this problem relate to dialects?

  • I think this problem happens regardless of the dialect.

Would you be willing to resolve this issue by submitting a Pull Request?

It seems to be a very difficult problem to solve correctly, meaning only replacing actual replacements, not just as string with a colon.

I also thought about an ignore replacement error, where it would not replace a string that doesn't match a replacement, but this would still allow for a soft "injection" if the user matched a existing replacement.

  • Yes, I have the time and I know how to start.
  • Yes, I have the time but I don't know how to start, I would need guidance.
  • No, I don't have the time, although I believe I could do it if I had the time...
  • No, I don't have the time and I wouldn't even know how to start.

Metadata

Metadata

Assignees

Labels

releasedtype: bugDEPRECATED: replace with the "bug" issue type

Type

No type

Projects

Status

Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions