This may not be a real issue since I have not analyzed what is going on under the hood, but won't having the node ID in the URL give the user complete control over which node gets passed as the parameter? Couldn't this be a security concern?

CommentFileSizeAuthor
#4 nodereference_url_nid_check.patch1.18 KBquicksketch

Comments

Leonth’s picture

No it won't be. Using the url we can refer to any node, but once you try to preview or submit the node Drupal will say "[label of the field]: this post can't be referenced."

I haven't checked it with more elaborate node list (nodes that are eligible to be referenced) using views, however.

kurzweil4’s picture

OK. So, if I understand correctly, nodes to which the user does not have access to, because of security, will cause an error message when they try to make references to them? If a user has access to create/access that content type, they would be able to make references to any node in that type wouldn't they?

I don't have a Drupal installation in front of me at the moment, so I am not sure about being able to set permissions regarding the ability to references other people's nodes, but one theoretical situation comes to mind, although this would (hopefully) not be implemented in reality: a user creates creates a bank transaction (content type) and the bank account (content type) node id is passed as a url parameter.

I am obviously exagerating here, as someone should not design an application like this, but this is one scenario in which you would not want a user to reference another user's node.

quicksketch’s picture

Yep, like Leontius says, the node form will throw an error when you try to save the page, so we're secure because the built-in Node Reference field validates for us. However, I agree that it would be good to prevent the option from being filled it at all. We should fall back to the empty option if the item in the URL does not validate. That is, if you've selected "Show 404 page" then if the URL doesn't validate you get the page not found.

quicksketch’s picture

Status: Active » Fixed
StatusFileSize
new1.18 KB

This patch checks the NID validity before showing the form in addition to the normal validation check added by Node Reference module on form submission. Though there wasn't a security concern before, this patch helps the usability of the module to give the user the error earlier instead of after they've filled out the entire form only to find they can't submit.

quicksketch’s picture

Title: Security Concern for Abritrary IDs » Use fallback behavior when URL NID is invalid
kurzweil4’s picture

Great!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.