← Back to team overview

openerp-community-reviewer team mailing list archive

Re: [Merge] lp:~serpentcs/web-addons/multi_image_7.0 into lp:web-addons

 

Review: Needs Fixing

Hello Friend.

I found the support for the security issue, "at least from my point of view".

Point 1:

167	+            file_name = current_dat_time + "_" + ufile.filename

With this line we brake a Python Standard, we have some libraries to manage the file names in a secure way, "Why use dates?".

One option:  

http://stackoverflow.com/questions/10501247/best-way-to-generate-random-file-names-in-python

AFAIK It is already implemented on document_ftp

Point 2:

Your module "without ask the user or/and the technicians" is writing over the web/module, it is a big security issue even with community addons, you can find the support about why name space correctly where you put your files.

168	+            addons_path = openerpweb.addons_manifest['web']['addons_path'] + "/web/static/src/img/image_multi/"

Option 1:
In this case you can use the static_http_document_root in your server config file, and if it is not setted ask for one with a raise may be.
Option 2:
Extend the document module giving the access to the folders to physical folders.

IOH, FYI, we are working on it, and we think it is a best approach, do you think we can merge the concepts in your module?


http://www.w3.org/Protocols/rfc2616/rfc2616-sec15.html [[Section 15.2]]

IMHO, it should be done extending the dms in OpenERP (with your approach of upload and make public), if you have some point against it it should be great if you share your thoughts.

For now and until we can not solve the security issues (or support them)

ICONS: Please use generic icons with a little mention to your company specifying the use of the module, avoind use your branding __only__

Descriptors.

Can you please explain a little how use the module in the descriptor?

Conclusion,

For me it is a great approach __really__ but it should be great if you can argue my points or improve in your code, please (or better put the branch with openerp-community owner and we can help ;-)  )
-- 
https://code.launchpad.net/~serpentcs/web-addons/multi_image_7.0/+merge/179857
Your team Web-Addons Core Editors is subscribed to branch lp:web-addons.


Follow ups