jadell / neo4jphp

PHP wrapper of the Neo4j REST interface
Other
532 stars 137 forks source link

Fix cypher injection in addLabels #139

Closed agix closed 10 years ago

agix commented 10 years ago

addLabels use rest cypher query uri instead of rest add label uri This introduce a nice Cypher injection in label name. This patch should resolve it using the good uri.

agix commented 10 years ago

Here is a proposition to patch the vulnerability I pointed out in Issue. Feel free to improve and/or comment !

agix commented 10 years ago

You know, I just provide a begining of patch trying to copy the way you handle every other REST calls. I didn't take the time to look after every cases nor every habit you have in the lib. Feel free to adapt it, I'm not a dev, my conscience is clear :D.

jadell commented 10 years ago

As a general rule, if you're going to contribute to open source software, you should take the time to familiarize yourself with the "habits" of the library, and make an effort to conform to them, even if you don't agree with them. You shouldn't just submit patches with the expectation that someone else will clean them up for you. And you should definitely not submit patches saying "this is how it should be done, now go finish it for me." Especially if you start the pull request with "Feel free to improve and/or comment !"

I'm more than willing to close whatever security vulnerability you find (though I still haven't seen an actual exploit of this particular implementation of this feature. Please send an example if you have one.) If you want to contribute meaningfully, you'll have my gratitude. But arrogance and nonchalance is not going to make anyone want to work with you.

jadell commented 10 years ago

Reference #138

agix commented 10 years ago

Yes my bad, I don't especially wanted to contribute. I was thinking spending some time to propose a solution to the security bug I reported was a good idea. So forget it. You are aware of the bug. Patch it or don't patch it, using this begining of patch or not.