Security vulnerability in 'debug' dependency

[larsvankleef]

Description:
There is a security vulnerability in one of the dependencies of aframe. It looks like there is no update coming form the debug packages that is currently used. Maybe this package will help you out: https://github.com/visionmedia/debug

source:
https://snyk.io/test/npm/debug/2.6.8

• A-Frame Version: current
• Platform / Device: all

[dmarcos]

Not sure if those npm warnings are worth considering. Debug is just a small logging utility library and we currently use a fork we maintain. Not sure what security vulnerability is the message referring to. Maybe spurious warning?

[EECOLOR]

Not sure if those npm warnings are worth considering.

To me it seems they are always worth considering.

Debug is just a small logging utility library and we currently use a fork we maintain.

You could rename your fork, that would at least not trigger vulnerability reports targeted to other libraries. Also, if you don't want to fix the vulnerability you should mention that the fork should not be used for public consumption in a node.js environment.

As for the threat to applications using aframe. If a bad user somehow is able to influence the information given to the debug library, he is able to make the site unresponsive. I don't know if there is any aframe code executed in server-side rendered situation.

In any case, if you ask me I would say the risk is tolerable to this project. I would just change the name of the library to prevent the 'wrong' vulnerability reports from popping up.

[hugopeixoto]

I don't think that the vulnerability is critical, but having github report a vulnerability as soon as you add A-Frame might be a good reason to try to get rid of the warning.

From what I understand, aframe uses the noTimestamps branch of the ngokevin/debug fork. Here's the PR that introduced it: #2550

Upstream is working on customizing the message format, so it'd be possible to remove the "+Xms" without the need of a fork.

I think this could also be fixed by bringing the fork up to date with the latest upstream.
I did the rebase in my local fork: hugopeixoto/debug@87dba31

I don't think I can just make a PR to ngokevin, because that would show up as 120 commits, and merging it directly to ngokevin/noTimestamps would cause the branch history to become confusing.

If you want to apply it, ngokevin would need to:

• fetch master from upstream
• create a new branch from upstream/master
• cherry pick my commit onto that branch, or let me make a PR targeting the branch
• point ngokevin/noTimestamp to that new branch (git reset --hard and push --force-with-lease)

[emericcolombe]

Hi guys,

I stumbled upon this is issue as well. I have to run my app through a dependency scanner that checks for vulnerabilities, and aframe pops up as vulnerable package because of this little patched debug dependency.
The github advisory is the following:
GHSA-gxpj-cx7g-858c

The noTimestamps branch of the ngokevin/debug made by @ngokevin was based on debug 2.2.0, and the vulnerability was fixed in debug 2.6.9.

@hugopeixoto already did all the rebase work, do you think we can merge this ?

[dmarcos]

@emericcolombe I tried the rebase and it doesn't work. debug messages don't print in console. Don't have the bandwidth to investigate but if someone wants to get it working happy to incorporate.

[arpu]

@dmarcos i tested with the original debug version from npm 4.3.4

it just works the only different is the +0ms in log

if only the message should be shown we can add debug-js/debug#582 (comment)

[dmarcos]

Thanks. Feel free to open a PR. If we can reproduce same behavior than we have today with a newer version.