Arbitrary Command Injection

[xstable]

Affected versions of this package are vulnerable to Arbitrary Command Injection. An attacker could inject arbitrary shell commands if the user input can influence the iface argument.

https://snyk.io/vuln/npm:macaddress:20180511

[zhaparoff]

https://nodesecurity.io/advisories/654
https://hackerone.com/reports/319467

[0xF48]

not sure but var iface = os.NetworkInterfaces() index.js is the one that is used, not lib/xxx. the lib/xxx is used by the library itself which is in index.js

[zhaparoff]

iface is passed as a first parameter to the '.one' method in the public API.
And is not sanitized in linux/unix/macos implementations.
Only windows one is safe, since does not pass it to syscalls directly.

[asifm]

@zhaparoff (I have no idea what iface is or what it is for.) Are you saying I don't need to worry if I have a Windows machine?

[jourdanrodrigues]

Just got this as output when installing my dependencies. Maybe flooding the issue, but I think more cases are always welcome.

https://go.npm.me/audit-guide

https://nodesecurity.io/advisories/654

[zhuangya]

create-react-app built projects are also infected.

[Logikgate]

Hey everyone, I opened a pull request #18 to resolve this. It is my understanding that by using execFile instead of exec you avoid command injection because the arguments are not run in a shell environment. I'd love some input on this though.

[francoabaroa]

CC: @Logikgate Awesome!! Thank you!

Here is another reference:

https://nodejs.org/api/child_process.html#child_process_child_process_execfile_file_args_options_callback

_"The child_process.execFile() function is similar to child_process.exec() except that it does not spawn a shell by default. Rather, the specified executable file is spawned directly as a new process making it slightly more efficient than child_process.exec().

The same options as child_process.exec() are supported. Since a shell is not spawned, behaviors such as I/O redirection and file globbing are not supported."_

Thanks again

[TixieSalander]

Looks like the maintainer(s) of this project haven't pushed a single commit since 3 years or even be active in the issues/PR lately.
It doesn't seem like a good omen for the projects who are using it.

Any alternative in mind?

[jourdanrodrigues]

@theotix We should really try contact him: http://www.scravy.de/

[TixieSalander]

@jourdanrodrigues Email sent

[Logikgate]

I had also reached out when I opened the PR, but since there are PR requests of more than a year old and no new commits to this repo I think we can safely assume it is abandoned.

With that in mind I have published a new npm module based on our fork. This fork will be maintained for security fixes and will be open to feature pull requests from the community.

You can use it with:
npm install macaddress-secure
or
yarn add macaddress-secure

Also, if you're using yarn you can force your sub-modules to use this package by adding the below to your package.json
"resolutions": { "*/**/macaddress": "macaddress-secure^0.2.11" }

[abolajibisiriyu]

@Logikgate I just tried using your suggestion but i got this error:
warning Resolution field "macaddress-secure^0.2.11" has an invalid version entry and maybe ignored

[transitive-bullshit]

NPM's support team is very responsive for issues like this; I'm guessing that if @scravy doesn't respond soon, they'll happily add anyone here as a collaborator to the original package given the large number of dependencies.

[martpie]

Only @scravy can add collaborators to this repo, not NPM. What npm can do is to attribute the name of the package to someone else.

[transitive-bullshit]

@KeitIG agreed; I'm just saying that instead of trying to get hundreds of dependents to move npm packages, we should try to get NPM to add @Logikgate or @theotix as collaborators to the npm package, so they can publish a fix to the original package.

[epet]

@transitive-bullshit, wouldn't the admin action need to be here on github and on npm allowing @Logikgate to publish a fixed package? Or are you saying only NPM that would allow @Logikgate to take over the npm package name macaddress?

[transitive-bullshit]

@epet the latter

[scravy]

I accepted pull #20 and released version 0.2.9.

Thank you all for having an eye on this package.