Backporting attributes vulnerability fix to v.6.x (CVE-2023-22578) #15694
-
Hi, our security scan reported a vulnerability in Sequelize v.6.28.0 (CVE-2023-22578). I think this is related to this pr: #15374, I tested with v.6.28.0 and confirmed that the vulnerability exists. The question is that are you going to backport the fix to v.6.x? |
Beta Was this translation helpful? Give feedback.
Replies: 3 comments 2 replies
-
Hi! At the present time I do not know. The problem is that, while I did make the changes to remove a footgun in v7, that behavior was not a bug but a deliberate feature implemented by one of the previous teams that worked on Sequelize, and there are codebases relying on it. Backporting #15374 would introduce a breaking change in a minor release. We've exceptionally done that in the past when we fixed a major issue (#14519), but I am not convinced that it's warranted for this one. The reason I think releasing this breaking change in a minor release is unwarranted is that it can only be a problem if you use user-provided values as the name of a column to select. Even without the fix, you should make sure that input is safe as the query will crash if the input is not a valid attribute name. We're stuck between a rock and a hard place. We can either introduce a breaking change in a minor release, or have a critically vulnerability warning. We need to check with the people that published that vulnerability if they would consider a big bold warning that that property accepts arbitrary SQL (and therefore that user data should not be used in it) to be sufficient to dismiss the audit warning Link to github advisory: GHSA-8mwq-mj73-qv68 |
Beta Was this translation helpful? Give feedback.
-
I've opened a PR to fix this issue: #15710. We opted for what we believe to be the lesser of the two evils |
Beta Was this translation helpful? Give feedback.
-
We have now released that change as Sequelize 6.29.0 |
Beta Was this translation helpful? Give feedback.
Hi!
At the present time I do not know.
The problem is that, while I did make the changes to remove a footgun in v7, that behavior was not a bug but a deliberate feature implemented by one of the previous teams that worked on Sequelize, and there are codebases relying on it.
Backporting #15374 would introduce a breaking change in a minor release. We've exceptionally done that in the past when we fixed a major issue (#14519), but I am not convinced that it's warranted for this one.
The reason I think releasing this breaking change in a minor release is unwarranted is that it can only be a problem if you use user-provided values as the name of a column to select. Even without the fix, you sh…