Hello All,
Recently working for a client I encountered a weird issue related to special characters encodings. We have product names containing special characters like ' (apostrophes). When we create orders for it, an encoded value for it is stored in OrderItem.itemDescription. The same encoded value also copied for invoice and return. When I checked the Product entity record, the original value (name without encoding) was stored there. I debugged the issue at code level and found that the system does encoding (string or html) at the time of order creation. I understand that for security reasons (and I want to know more about it), the system does the encodings. My concerns are related to not using encoding when we create products. And it is not good UI experience to display encoded forms of values to screens. I suggest we should use some methods to display encoded values properly on screens or remove the encoding at the very first place. Please feel free to provide any suggestions or inputs. -- Kind Regards, Amit Gadaley *Technical Consultant* *HotWax Systems* *Enterprise open source experts* cell: +91-95845-93069 office: 0731-409-3684 http://www.hotwaxsystems.com |
Administrator
|
Hi Amit,
It's better to encode to prevent XSS. Then of course we need to decode when showing those values. Actually in this case it's automatically encoded by Freemarker as explained in this old but still good reference: https://ofbiz.markmail.org/thread/e2iznsqhhxxdplxh So we can do the same, ie using StringUtil.wrapString(), like <input size="60" type="text" name="description_${cartLineIndex}" value="${StringUtil.wrapString(cartLine.getName(dispatcher))?default("")}"/><br /> This should be done everywhere it's needed in FTL files. I have added a patch for similar "cartLine.get...()" cases at OFBIZ-12029. Of course other cases like that can pop out anytime; eg, I'll also fix a long awaiting one at OFBIZ-7343... We could think that using Freemarker autoescaping as suggested in OFBIZ-7675 would be better. But escaping is not encoding. You can check by using ?html (local autoescaping ) instead of StringUtil.wrapString(). You get "tes&#39;t" For widgets forms, there is a problem currently investigated with OFBIZ-12026... HTH Jacques Le 26/09/2020 à 11:00, Amit Gadaley a écrit : > Hello All, > > Recently working for a client I encountered a weird issue related to > special characters encodings. We have product names containing special > characters like ' (apostrophes). When we create orders for it, an encoded > value for it is stored in OrderItem.itemDescription. The same encoded value > also copied for invoice and return. When I checked the Product entity > record, the original value (name without encoding) was stored there. I > debugged the issue at code level and found that the system does encoding > (string or html) at the time of order creation. > > I understand that for security reasons (and I want to know more about it), > the system does the encodings. My concerns are related to not using > encoding when we create products. And it is not good UI experience to > display encoded forms of values to screens. > > I suggest we should use some methods to display encoded values properly on > screens or remove the encoding at the very first place. > > Please feel free to provide any suggestions or inputs. > |
Hi Amit,
I agree with Jacques. Though I see that in shopping cart implementation when copying product name to order item name it uses string encoding vs html encoding, I think this could be fixed to use html encoding for product/item name like it's done for product/item description in the same method. Thanks. Mridul Pathak On Mon, Sep 28, 2020 at 2:21 PM Jacques Le Roux < [hidden email]> wrote: > Hi Amit, > > It's better to encode to prevent XSS. Then of course we need to decode > when showing those values. > Actually in this case it's automatically encoded by Freemarker as > explained in this old but still good reference: > https://ofbiz.markmail.org/thread/e2iznsqhhxxdplxh > > So we can do the same, ie using StringUtil.wrapString(), like > > <input size="60" type="text" name="description_${cartLineIndex}" > value="${StringUtil.wrapString(cartLine.getName(dispatcher))?default("")}"/><br > /> > > This should be done everywhere it's needed in FTL files. > > I have added a patch for similar "cartLine.get...()" cases at OFBIZ-12029. > Of course other cases like that can pop out anytime; eg, I'll also fix a > long awaiting one at OFBIZ-7343... > > We could think that using Freemarker autoescaping as suggested in > OFBIZ-7675 would be better. > But escaping is not encoding. You can check by using ?html (local > autoescaping ) instead of StringUtil.wrapString(). You get > "tes&#39;t" > > For widgets forms, there is a problem currently investigated with > OFBIZ-12026... > > HTH > > Jacques > > Le 26/09/2020 à 11:00, Amit Gadaley a écrit : > > Hello All, > > > > Recently working for a client I encountered a weird issue related to > > special characters encodings. We have product names containing special > > characters like ' (apostrophes). When we create orders for it, an encoded > > value for it is stored in OrderItem.itemDescription. The same encoded > value > > also copied for invoice and return. When I checked the Product entity > > record, the original value (name without encoding) was stored there. I > > debugged the issue at code level and found that the system does encoding > > (string or html) at the time of order creation. > > > > I understand that for security reasons (and I want to know more about > it), > > the system does the encodings. My concerns are related to not using > > encoding when we create products. And it is not good UI experience to > > display encoded forms of values to screens. > > > > I suggest we should use some methods to display encoded values properly > on > > screens or remove the encoding at the very first place. > > > > Please feel free to provide any suggestions or inputs. > > > |
Administrator
|
Hi Mridul,
Maybe I miss what you mean, because everywhere, not only in ShoppingCartItem.java, "html" encoding is used not "string" encoding. Again, the reason is to prevent XSS attacks. I understand that you suggest that, eg: String productName = ProductContentWrapper.getProductContentAsText(product, "PRODUCT_NAME", this.locale, dispatcher, "string"); instead of currently String productName = ProductContentWrapper.getProductContentAsText(product, "PRODUCT_NAME", this.locale, dispatcher, "html"); Do you mean something else? If it's what you mean then we should not change. Because else it's easy to replace a product/item name at https://localhost:8443/ordermgr/control/orderentry by <script>alert('xss')</script> <https://localhost:8443/ordermgr/control/product?product_id=testQuote> Then if you finalize the order and get to https://localhost:8443/ordermgr/control/processorder the new product/item name is showing and it you click on it you get to https://localhost:8443/ordermgr/control/product?product_id=yourProductId ans get an "XSS" attack. If you mean something else, please explain Thanks Jacques Le 28/09/2020 à 16:44, Mridul Pathak a écrit : > Hi Amit, > > I agree with Jacques. Though I see that in shopping cart implementation > when copying product name to order item name it uses string encoding vs > html encoding, I think this could be fixed to use html encoding for > product/item name like it's done for product/item description in the same > method. > > Thanks. > Mridul Pathak > > On Mon, Sep 28, 2020 at 2:21 PM Jacques Le Roux < > [hidden email]> wrote: > >> Hi Amit, >> >> It's better to encode to prevent XSS. Then of course we need to decode >> when showing those values. >> Actually in this case it's automatically encoded by Freemarker as >> explained in this old but still good reference: >> https://ofbiz.markmail.org/thread/e2iznsqhhxxdplxh >> >> So we can do the same, ie using StringUtil.wrapString(), like >> >> <input size="60" type="text" name="description_${cartLineIndex}" >> value="${StringUtil.wrapString(cartLine.getName(dispatcher))?default("")}"/><br >> /> >> >> This should be done everywhere it's needed in FTL files. >> >> I have added a patch for similar "cartLine.get...()" cases at OFBIZ-12029. >> Of course other cases like that can pop out anytime; eg, I'll also fix a >> long awaiting one at OFBIZ-7343... >> >> We could think that using Freemarker autoescaping as suggested in >> OFBIZ-7675 would be better. >> But escaping is not encoding. You can check by using ?html (local >> autoescaping ) instead of StringUtil.wrapString(). You get >> "tes&#39;t" >> >> For widgets forms, there is a problem currently investigated with >> OFBIZ-12026... >> >> HTH >> >> Jacques >> >> Le 26/09/2020 à 11:00, Amit Gadaley a écrit : >>> Hello All, >>> >>> Recently working for a client I encountered a weird issue related to >>> special characters encodings. We have product names containing special >>> characters like ' (apostrophes). When we create orders for it, an encoded >>> value for it is stored in OrderItem.itemDescription. The same encoded >> value >>> also copied for invoice and return. When I checked the Product entity >>> record, the original value (name without encoding) was stored there. I >>> debugged the issue at code level and found that the system does encoding >>> (string or html) at the time of order creation. >>> >>> I understand that for security reasons (and I want to know more about >> it), >>> the system does the encodings. My concerns are related to not using >>> encoding when we create products. And it is not good UI experience to >>> display encoded forms of values to screens. >>> >>> I suggest we should use some methods to display encoded values properly >> on >>> screens or remove the encoding at the very first place. >>> >>> Please feel free to provide any suggestions or inputs. >>> |
Administrator
|
Le 01/10/2020 à 08:42, Jacques Le Roux a écrit :
> by <script>alert('xss')</script> <https://localhost:8443/ordermgr/control/product?product_id=testQuote> This has been inadvertently copied there (actually copied from HTML browser source page when copying <script>alert('xss')</script>, I'm a leazy type-writer) |
Free forum by Nabble | Edit this page |