Cypher injection when setting labels to a node

[agix]

https://github.com/jadell/neo4jphp/blob/master/lib/Everyman/Neo4j/Command/SetLabels.php#L45

You should not use REST Cypher URI to add label to a node. http://docs.neo4j.org/chunked/milestone/rest-api-node-labels.html

This vulnerability could be exploited to do really bad things if user get control of a label.

$a_label = $client->makeLabel('Articles'); $add_label = $client->makeLabel($_GET['label']);

$articles = $a_label->getNodes('Title',$_GET['title']);

if(count($articles)==1){ $article = $articles[0]; } else{ $article = $client->makeNode(); $article->setProperty('Title', $_GET['title']) ->setProperty('Description', $_GET['description']) ->save(); }

$article->addLabels(array($a_label,$add_label)); echo $article->getProperty('Description');

I'm writing an article on Neo4j security, I will publish it soon and I cite this vulnerability.

[jadell]

I'm not understanding what the vulnerability in neo4jphp is here. You shouldn't be passing $_GET values directly to any data store without sanitizing them first. That is a general rule for all PHP applications.

Also, the reason for using Cypher instead of the API endpoints was to work around a few bugs in the Labels API endpoints.

[agix]

It's your method that directly speak to the data store no one could guess that an array of Labels objects will be converted in cypher query without sanitization...

I just point it, I proposed a patch, do as you want, it's more work for pentester...

[jadell]

Did you actually manage to exploit this somehow? Can you post an example?

[agix]

In the exemple I posted (seems realistic to let user add his own label name to his article as a tag for exemple). So in this case title=First&label=my_label%0aWITH%20n%20MATCH%20(s:Secret)%0aCREATE%20(m:Articles%20{%20Title%20:%20%27Injected%27,%20Description%20:%20s.Password})%20RETURN%20labels(n)%20as%20labels;//`

You can set a description with a password from another node for exemple. You can also delete every nodes.

[jadell]

@agix I just created a branch called safe-escape-labels that I believe patches the issue you found. Would you mind running your attack against that branch and making sure the vulnerability is closed? As soon as you say it is fixed, I will merge the fix into master. Thanks.

[agix]

Nice, I check it now.

[agix]

Should be right...