The module only removes the "add to cart" button but that not prevent a product to be actually added to cart.
This is a critical bug.
The solution is to add the following code at the end of uc_notforsale.module:

<?php
/**
 * Implementation of hook_add_to_cart().
 */
function uc_notforsale_add_to_cart($nid, $qty, $data) {
  if (_uc_notforsale_isnotforsale($nid)) {
    $result[] = array('success' => FALSE);
    return $result;
  }
}
?>
CommentFileSizeAuthor
#3 764650.uc_notforsale.prevent-adding-to-cart.patch1.19 KBjoachim

Comments

anrikun’s picture

anrikun commented
Title: Security vulnerability: module does not actually prevent a not for sale to be added to cart » Security vulnerability: module does not actually prevent a "not for sale" product to be added to cart
dandaman’s picture

dandaman commented
Status: Needs review » Reviewed & tested by the community

This bug does exist and this code does prevent you from adding it to the cart.

joachim’s picture

joachim commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new 764650.uc_notforsale.prevent-adding-to-cart.patch1.19 KB

I'm curious -- how can you add a product to your cart without the 'Buy button'? (#765730: "Buy It Now" button does not get removed from my views. has just been fixed :)

At any rate, it's a sensible precaution and it's even something I put in the todo list...

Committed this patch.

#764650 by anrikun: Fixed 'not for sale' product able to be added to the cart.

Thanks both of you for the code and the testing :D

Edit: new release available :)

anrikun’s picture

anrikun commented

That means that if #765730: "Buy It Now" button does not get removed from my views. had been fixed 3 days earlier, I would not have any chance to discover the security bug :-O!

Thank you for this new release.
There is still a minor bug though: it seems that Views is not aware that price and buyitnow button fields are empty, as even if the fields values are not shown themselves, the markup wrapping them is still rendered. This can lead to theming problems (when using background styles for instance).
There is a way to workaround this using preprocessing and adding a flag relationship to the view, but it would be great if it was fixed in the module itself.
May I add this as a new separate issue?

Another thing: this so useful module is too hard to find from Google. Please could you add a few more keywords (like "disable product") inside the project page so that users can find it more easily?

joachim’s picture

joachim commented

> May I add this as a new separate issue?

You can, but I'm not sure there is much that can be done by this module, as the Views handlers are out of our hands. But worth filing to discuss workarounds at any rate.

> Please could you add a few more keywords (like "disable product") inside the project page so that users can find it more easily?

Sure. If you think of others, please file a new issue for that too :)

dandaman’s picture

dandaman commented

The reason I said that it doesn't prevent was because in my Views it was still printing a button until we fixed that in 6.x-1.2. It took the "Add to Cart" button away, but the Views still had the button and it would still add it. Thanks for looking at these and making changes.

Status: Fixed » Closed (fixed)

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