Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SECURITY] event-stream incident #268

Closed
dhaavi opened this issue Nov 28, 2018 · 10 comments
Closed

[SECURITY] event-stream incident #268

dhaavi opened this issue Nov 28, 2018 · 10 comments
Labels
priority/high Anything which is in high priority type/build Anything related to the build process
Milestone

Comments

@dhaavi
Copy link

dhaavi commented Nov 28, 2018

There was a security issue with the npm package event-stream.

Original issue: dominictarr/event-stream#116 (comment)
Semantic issue: Semantic-Org/Semantic-UI#6687

Please update event-stream to version 3.3.4:

By this time fixes are being deployed and npm has yanked the malicious version. Ensure that the developer(s) of the package you are using are aware of this post. If you are a developer update your event-stream dependency to [email protected]. This protects people with cached versions of event-stream.

@y0hami y0hami added priority/high Anything which is in high priority type/build Anything related to the build process labels Nov 28, 2018
@y0hami
Copy link
Member

y0hami commented Nov 28, 2018

We are aware of the issue and are trying to come up with a fix. event-stream is in a package called prompt-sui which is maintained by the author of SUI who is currently AFK from development it seems so we don't have an easy way to update the version.

@dhaavi
Copy link
Author

dhaavi commented Nov 28, 2018

I see. Thank you for the information.

Is there a way to override this dependency? It should be fully compatible as I understand.

y0hami pushed a commit that referenced this issue Nov 28, 2018
Change `event-stream` version to fix security exploit.
Refs:
  dominictarr/event-stream#116
  dominictarr/event-stream#115

Closes #268
@y0hami
Copy link
Member

y0hami commented Nov 28, 2018

@dhaavi See the PR I just created. #269

@dhaavi
Copy link
Author

dhaavi commented Nov 28, 2018

Looks great!
(I am not experienced with node, so I can't tell you how effective this fix is.)

y0hami pushed a commit that referenced this issue Nov 28, 2018
Change `event-stream` version to fix security exploit.
Refs:
  dominictarr/event-stream#116
  dominictarr/event-stream#115

Closes #268
@y0hami y0hami added the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Nov 28, 2018
@etshy
Copy link

etshy commented Nov 28, 2018

what if I do npm update ? The lock file block the maximum version to 3.3.4 or it will download the 3.3.6 (which is infected if I remember correctly the version number) ?

If the package-lock works the same as composer.lock, it will force this version on install only, If I'm not mistaken. On update it will search dor the highest version that match with the package.json

I also saw this issue yesterday, and seems some packages are updating to 4.0.1 (when the flatmap dep was removed)

@dhaavi
Copy link
Author

dhaavi commented Nov 28, 2018

it will download the 3.3.6 (which is infected if I remember correctly the version number) ?

No, because

npm has yanked the malicious version

The version change is there to defeat caches. For example, if you've downloaded the infected version and do git pull and npm ... it should remove the infected version cached locally.

@etshy
Copy link

etshy commented Nov 28, 2018

Oh ok, I missed the information about npm removing the malicious versions.

Nice to know it should be fixed. I don't work with crypto but still good to not have malicious code.

@lubber-de lubber-de added this to the 2.7.0 milestone Nov 28, 2018
@Atulin
Copy link

Atulin commented Nov 28, 2018

Wouldn't it be a good idea to think of removing prompt-sui in the future? From what I've seen, it's vanity stuff at best.

Overall, I'd consider removing as many dependencides as possible, but that's a matter for another discussion.

@y0hami
Copy link
Member

y0hami commented Nov 28, 2018

@Atulin We currently have plans to rewrite the build process when we do 3.0 but that is a whole other project. When we do this we will be getting rid of a lot of the dependencies.

@Atulin
Copy link

Atulin commented Nov 28, 2018

That + removing the dependency on jQuery would be a dream come true. Glad there are steps being taken in that direction 👌

y0hami pushed a commit that referenced this issue Dec 18, 2018
Change `event-stream` version to fix security exploit.
Refs:
  dominictarr/event-stream#116
  dominictarr/event-stream#115

Closes #268
This was referenced Dec 21, 2018
@y0hami y0hami closed this as completed in 4eedd99 Dec 21, 2018
@lubber-de lubber-de removed the tag/next-release/nightly Any issue which has a corresponding PR which has been merged and is available in the nightly build label Dec 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/high Anything which is in high priority type/build Anything related to the build process
Projects
None yet
Development

No branches or pull requests

5 participants