potteryhouse / stock_by_attribute_1.5.4

Stock by Attribute for Zen Cart 1.5.3
GNU General Public License v2.0
2 stars 5 forks source link

Should apply patch "XSS problem for unsanitized comment field" #46

Closed cstdenis closed 8 years ago

cstdenis commented 8 years ago

The modified includes/modules/pages/checkout_confirmation/header_php.php should be patched to have the recently announced security patch.

Announcement: https://www.zen-cart.com/showthread.php?218914-Security-Patches-for-v1-5-4-November-2015 Patch: https://github.com/zencart/zc-v1-series/pull/563/files

Also, this file has screwed up line endings

/**^M^M
 * checkout_confirmation header_php.php^M^M
 *^M^M
 * @package page^M^M
mc12345678 commented 8 years ago

Already applied at: https://github.com/mc12345678/Stock_By_Attributes_Combined; however, that needs to make it to both 1.5.4 and 1.5.3... Plan at the moment is to have 1.5.3 thread point to this one, and update this one to be the combination of the two unfortunately will have to update the version of the software without updating the URI of the software... Though I guess I could call it 1.5.4.1.

Maintaining two separate areas that have 90% of the same code is getting ridiculous... As to line endings... Will see what can be done to correct them as github upload software does occasionally identify an issue with them when saving the updates.. Hopefully will be easy to correct...

cstdenis commented 8 years ago

Thanks for replying so fast. It's always nice to see issues actually get a reply (far too many semi-abandoned projects out there).

It looks like you were able to combine 1.5.3 and 1.5.4 in the other repo. There shouldn't be too much code differences between them anyway -- zc154_compatibility.php should handle the most annoying of them. Could close (phase out) the 1.5.4 and 1.5.3 repos and just leave a link in them to the combined one--no point making more work for yourself.

1.5.1 could just be semi-abandoned other than security patches -- make people upgrade their zencart if they want new features.

This mod touching so many core files really makes it a pain -- both for end users to install (conflicts with other mods) and for you to maintain. Looks like you are improving things by making more use of notify/observers but unfortunately there are not hooks for everything.

Regarding the line endings, if the github client software is the problem, maybe using the standard command line git to delete and re-add the file or something could clear it up.

Anyway, this issue was just to make you aware of those 2 minor issues.

mc12345678 commented 8 years ago

Welcome, try to stay abreast of things and even if not my creation, help where/when I can. In the spirit of ZC, 1.5.1 continues to be considered for maintaining operation, especially with the issues that are being resolved but exist for those upgrading. Yes, eventually it will fall by the waist side, but not right now.

Some of the hooks can't be fully utilized because of the nature of the data that needs to be passed along/used and really trying to use what ZC offers and trying to suggest where other hooks might be of benefit. I'm also trying to reduce the load on files that are frequently modified to support other plugins so that integration can be a little easier (ie. includes/modules/YOUR_TEMPLATE/attributes.php). There are/have been a few other recommended hooks for the attributes.php file which I plan to eventually incorporate, but in the mean time there is still some other functionality that needs to be injected so that this is robust enough for the majority of people.

I sure wish I could remember which option I selected when I was asked about modifying the line ending and the "path" that the file took to end up being "double-spaced"... I don't want to repeat it... :) Looks horrible, makes upgrading a pain, etc...

The issue remains open until fully resolved/incorporated here. Thank you for piping up!

mc12345678 commented 8 years ago

Closing this out for now. If file endings remains a problem (haven't been able to identify them as an issue yet on my side) please open a new issue.

Trying to update and clean up the issues list to ensure properly focused.