webpack/webpack-dev-serverPublic

NotificationsYou must be signed in to change notification settings
Fork1.5k
Star7.9k

Fix security issues reported by Dependabot for version 4#5514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Draft

kretajak wants to merge3 commits intowebpack:version-4

base:version-4

Choose a base branch

fromkretajak:fix/security-issues

Draft

Fix security issues reported by Dependabot for version 4#5514

kretajak wants to merge3 commits intowebpack:version-4fromkretajak:fix/security-issues

Conversation

Copy link

kretajak commentedJun 6, 2025

This is abugfix
This is afeature
This is acode refactor
This is atest update
This is adocs update
This is ametadata update

For Bugs and Features; did you add new tests?

Fixes Security issues present in version 4 ofwebpack-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 commitsd2575ad and5c9378b

Additional Info

sapphi-red added2 commits

June 6, 2025 14:23

fix(security): requests with an IP addresses Origin header are not al…

5ba835f

…lowed to connect to WebSocket

fix(security): check cross origin requests

ba2e692

kretajak requested review fromalexander-akait,anshumanv,hiroppy andsnitin315 ascode owners

June 6, 2025 12:52

alexander-akait reviewed

Jun 6, 2025

View reviewed changes

lib/Server.js

		constheaders=
		/*@type {{ [key: string]: string \| undefined }} /
		(req.headers);
		if(

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Recently we added some fixes here to skip check forallowedHost, please add it here

Copy link

Author

kretajakJun 9, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It seems not that straightforward. To me bigger effort is needed to incorporate changes from03d1214. 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.

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@kretajak let's include03d1214 here, otherwise in some cases it will be impossible to setup a new version and we can merge

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@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.

Copy link

Author

kretajakJun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm currently trying to backport that commit. I'll inform you whether I was able to make it.

Copy link

Author

kretajakJun 11, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I have added new commit which should move us a bit closer. Seems like also changes from6045b1e might be required as they are tightly connected to03d1214.

@slorber feel free to continue the work.

slorber mentioned this pull request

Jun 6, 2025

chore(deps): bump webpack-dev-server from 4.15.2 to 5.2.1facebook/docusaurus#11239

Closed

Copy link

slorber commentedJun 6, 2025

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 18like 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)

facebook/docusaurus#11252 (comment)

MrBMT mentioned this pull request

Jun 9, 2025

Backport security fixes from v5.2.1 to v4#5515

Open

mertssmnoglu mentioned this pull request

Jun 10, 2025

chore(deps): Bump docusaurus to 3.8.1 | WIPticaretistatistik/ticaretistatistik.github.io#60

Closed

slorber mentioned this pull request

Jun 11, 2025

@docusaurus/code latest version used webpack-dev-server@4.15.2, which has Security Vulnerabilityfacebook/docusaurus#11256

Closed

7 tasks

Copy link

pikachugb commentedJun 11, 2025

Hello :) Is there an ETA for the release of potentially version 4.15.3 with the changes from this PR?

Copy link

linux-foundation-easyclabot commentedJun 11, 2025•
edited
Loading

The committers listed above are authorized under a signed CLA.

✅ login: sapphi-red / name: 翠 (5ba835f,ba2e692)
✅ login: alexander-akait / name: Alexander Akait (8de7782)

kretajak commented

Jun 11, 2025

View reviewed changes

lib/Server.js

		hostname==="localhost"\|\|
		hostname.endsWith(".localhost")\|\|
		hostname===this.options.host
		:true;

Copy link

Author

kretajakJun 11, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I believe there is a bug in version 5 ofweboack-dev-server.false is returned there, but my guess is that whenvalidateHost isfalse we should bypass checking and returntrue here.

Above function when called:isValidHost({ host: '127.0.0.1 }, 'host', validateHost = false) returnfalse while it should returntrue. I assume that is the reason so many tests of6045b1e were changed.

Copy link

Member

alexander-akaitJun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@kretajak it is not a bug, because 127.0.0.1 can be used for attack, you should manually set127.0.0.1 inallowedHosts for CORS requests, i.e. you openedbad-site.com, this this site can try to connect tows://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)

Copy link

Author

kretajakJun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Okey. I reverted it tofalse.

Copy link

Member

alexander-akait commentedJun 11, 2025

@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 thefix/security-issues branch from3d925e1 tod6be469Compare

June 11, 2025 20:13

alexander-akait approved these changes

Jun 12, 2025

View reviewed changes

fix: respect theallowedHostsoption for cross-origin header check (w…

8de7782

…ebpack#5510)

kretajak force-pushed thefix/security-issues branch fromd6be469 to8de7782Compare

June 12, 2025 11:39

kretajak marked this pull request as draft

June 12, 2025 11:39

Copy link

Author

kretajak commentedJun 12, 2025

I have converted it to draft as it's incomplete.

Copy link

wissemayadi21 commentedJun 16, 2025

hello please when this version will be published ?

Copy link

dharaneesh127 commentedJun 17, 2025

@hiroppy @anshumanv @snitin315 could you guys please review the PR, and if good can it be published ?

Copy link

Author

kretajak commentedJun 17, 2025•
edited
Loading

As I wrote here:#5514 (comment) backporting these extra changes is not straightforward. I would recommend dropping the last commit and merge this PR with the very first two commits, as they are essentially fixing the security issue.