Fix security issues reported by Dependabot for version 4 #5514
Conversation
…lowed to connect to WebSocket
kretajak
requested review from
alexander-akait,
hiroppy,
snitin315 and
anshumanv
as code owners
| const headers = | ||
| /** @type {{ [key: string]: string | undefined }} */ | ||
| (req.headers); | ||
| if ( |
There was a problem hiding this comment.
Recently we added some fixes here to skip check for allowedHost, please add it here
There was a problem hiding this comment.
It seems not that straightforward. To me bigger effort is needed to incorporate changes from 03d1214. This PR uses functions defined in previous commits not available in line 4.
If it's not a problem, I would consider merging this PR and creating separate PR for task you mentioned.
There was a problem hiding this comment.
@kretajak let me know if you need help backporting that commit. I could try to add it on top of your existing PR if you don't have time to. We have at least 3 Docusaurus issues asking us to solve this security warning so happy to help and get this solved asap.
There was a problem hiding this comment.
I'm currently trying to backport that commit. I'll inform you whether I was able to make it.
|
Thanks, we'd also appreciate a backport for Docusaurus because our current minor supports Node 18.0, incompatible with dev server v5, and all newly initialized Docusaurus sites will get dev server v4. We could bump to the latest Node 18 like Astro did recently (since it reached end of life) but if it's possible to avoid that it's better to not force our users to upgrade Node.js when upgrading a minor version (and I'd rather not release a new major version just for that security fix) |
|
Hello :) Is there an ETA for the release of potentially version 4.15.3 with the changes from this PR? |
| hostname === "localhost" || | ||
| hostname.endsWith(".localhost") || | ||
| hostname === this.options.host | ||
| : true; |
There was a problem hiding this comment.
I believe there is a bug in version 5 of weboack-dev-server. false is returned there, but my guess is that when validateHost is false we should bypass checking and return true here.
Above function when called: isValidHost({ host: '127.0.0.1 }, 'host', validateHost = false) return false while it should return true. I assume that is the reason so many tests of 6045b1e were changed.
There was a problem hiding this comment.
@kretajak it is not a bug, because 127.0.0.1 can be used for attack, you should manually set 127.0.0.1 in allowedHosts for CORS requests, i.e. you opened bad-site.com, this this site can try to connect to ws://localhost:3000 and without such headers non chromium and old chromium browsers will connect to your websockets and can take your source code (in some cases)
|
@kretajak Can you change your email in the last commin, CLA is failed, we can't merge commits without CLA @pikachugb This week |
kretajak
force-pushed
the
fix/security-issues
branch
from
3d925e1 to
d6be469
Compare
kretajak
force-pushed
the
fix/security-issues
branch
from
d6be469 to
8de7782
Compare
|
I have converted it to draft as it's incomplete. |
For Bugs and Features; did you add new tests?
Fixes Security issues present in version 4 of
webpack-dev-server. Similar fixes were already merged into version 5 ofwebpack-dev-server.Motivation / Use-Case
Fix issues reported by Dependabot:
Breaking Changes
It is breaking change but it's security wise. Similar changes are already in 5.x.x branch. See commits d2575ad and 5c9378b
Additional Info