10 Rules for Safer Code

Public Channel / Odoo Experience 2016

This presentation describes the most common security mistakes found in Odoo modules by the Odoo Security Team, and gives 10 rules that would help avoid them.
If you're familiar with the OWASP top 10, this is like Odoo's own top 10.

Share on Social Networks

Share Link

Use permanent link to share in social media

Share with a friend

Please login to send this presentation by email!

Embed in your website

Select page to start with

44. security@odoo.com

29. Avoid leaking user cookies and Session ID

2. Yet another security issue?!

22. Browsers blur the distinction between code and data!

17. Learn the API to avoid hurting yourself and other people !

33. Do you think this form is safe from CSRF attacks? As of Odoo 9, HTTP POST controllers are CSRF-protected Do not bypass it with GET controllers that act like POST!

10. R ULE #2 YOU SHALL NOT PICKLE Don’t use it. Ever. Use JSON.

1. 10 RULES FOR SAFER CODE Olivier Dony @odony

34. R ULE #8 MASTER thE RULES Odoo ACL and Rules are not trivial , be sure to understand them

9. 50% of vulnerabilities found every year include remote code execution injected via unsafe eval

32. R ULE #7 CSRF tokens FOR website forms HTTP Posts require CSRF tokens since v9.0

30. Do not over-Sudo IT R ULE #6 Review 2x your sudo() usage, particularly controllers/public methods

31. Do you think this form is safe? Not if it blindly takes the form POST parameters and calls write() in sudo mode!

15. R ULE #3 USE THE CURSOR WISELY Use the ORM API . And when you can’t, use query parameters .

21. R ULE #4 Fight XSS So many XSS vectors – gotta watch ’em all

13. Pickle is unsafe Seriously. >>> yummy = "cos\nsystem\n(S'cat /etc/shadow | head -n 5'\ntR.'\ntR." >>> pickle.loads(yummy) root:$6$m7ndoM3p$JRVXomVQFn/KH81DEePpX98usSoESUnml3e6Nlf.:14951:0:99999:7::: daemon:x:14592:0:99999:7::: (...) >>>

27. R ULE #5 GUARD PASSWORDS & tokens fiercely Secure all user and API tokens , and don’t leak them

42. R ULE #10 Do NOT open (), urlopen (), requests.post (), ... an arbitrary URL/Path! OPEN WITH CARE

4. “ eval” breaks the barrier between code and data Is this safe? Maybe ... it depends. Is it a good idea? No , because eval() is not necessary!

40. R ULE #9 There are better and safer alternatives Getattr is NOT YOUR FRIEND

14. Use JSON instead! json.dumps({ “widget” : “monetary” }) == '{"widget": "monetary"}' + safe + portable + human-readable

11. “ Warning : The  pickle  module is  not intended  to be secure  against erroneous or maliciously  constructed data.  Never unpickle  data  received from an  untrusted or  unauthenticated source . ”

12. Python’s pickle serialization is: + unsafe + not portable + human-unreadable pickle.dumps({ “widget” : “monetary” }) == "(dp0\nS'widget'\np1\nS'monetary'\np2\ns." Actually a stack-based language!

43. Summary 1. Eval is evil 2. You shall not pickle 3. Use the cursor wisely 4. Fight XSS 5. Guard passwords & tokens fiercely 6. Do not over-sudo it 7. CSRF tokens for weBsite forms 8. master THE rules 9. Getattr is not your friend 10. OPEN WITH care

16. SQL injection is a classical privilege escalation vector self.search(domain) The ORM is here to help you build safe queries: Psycopg can also help you do that , if you tell it what is code and what is data : query = ”””SELECT * FROM res_partner WHERE id IN %s ””” self._cr.execute( query , (tuple(ids),) ) SQL code SQL data parameters

3. R ULE #1 EVAL is EVIL Don’t trust strings supposed to contain expressions or code ( e v e n y o u r o w n ! )

24. Most XSS errors are trivial: DOM manipulations (JQuery) $elem.html(value) vs $elem.text(value) Only use it to insert HTML code that has been prepared and escaped by the framework. Never use it to insert text . For everything else , use: • t-esc : variables, URL parameters, ... • t-field : record data

41. Do NOT do this: def _get_it( self , field = ’partner_id’ ): return getattr(record, field) Try this instead: def _get_it( self , field = ’partner_id’ ): return record[field] By passing arbitrary field values, an attacker could gain access to dangerous methods! This will only work with valid field values

8. If you must eval parameters use a safe eval method J A V A S C R I P T // py.js is included by default py.eval( ’foo’ , { ’foo’ : 42 }); // require(”web.pyeval”) for // domains/contexts/groupby evaluation pyeval.eval( ’domains’ , my_domain); Do not use the built-in JS eval!

23. Most XSS errors are trivial: QWeb templates t-raw vs t-esc / t-field Only use it to insert HTML code that has been prepared and escaped by the framework. Never use it to insert text . For everything else , use: • t-esc : variables, URL parameters, ... • t-field : record data

25.     @http.route('/web/binary/upload', type='http', auth="user")     @serialize_exception     def upload(self, callback , ufile):         out = """<script language="javascript" type="text/javascript">                     var win = window.top.window;                     win.jQuery(win).trigger ( %s, %s );                 </script>"""   # (...)               return out % (json.dumps( callback ), json.dumps(args)) Some XSS are less obvious: callbacks JSON escaping is not sufficient to prevent XSS, because of the way browsers parse documents! /web/binary/upload?callback=</script><script>alert(’This works!');//

26. Users can often upload arbitrary files : contact forms, email gateway, etc. Upon download, browsers will happily detect the file type and execute anything that remotely looks like HTML, even if you return it with an image mime-type ! Some XSS are less obvious: uploads <svg xmlns = "http://www.w3.org/2000/svg" > <script type = "text/javascript" >alert( ’XSS!!’ );</script> </svg> A valid SVG image file!

6. There are safer and smarter ways to parse data in JAVASCRIPT “ 42” parseInt (x) parseFloat (x) Given this string Parse it like this “ [1,2,3, true ]” JSON .parse(x) ‘ { “ widget ” : “ monetary ” }’

18. This method is vulnerable to SQL injection def compute_balance_by_category ( self , categ = ’in’ ): query = ”””SELECT sum(debit-credit) FROM account_invoice_line l JOIN account_invoice i ON (l.invoice_id = i.id) WHERE i.categ = ’%s_invoice’ GROUP BY i.partner_id ””” self._cr.execute(query % categ) return self._cr.fetchall() What if someone calls it with categ = ”””in_invoice’; UPDATE res_users SET password = ’god’ WHERE id=1; SELECT sum(debit-credit) FROM account_invoice_line WHERE name = ’”””

19. This method is still vulnerable to SQL injection def _compute_balance_by_category ( self , categ = ’in’ ): query = ”””SELECT sum(debit-credit) FROM account_invoice_line l JOIN account_invoice i ON (l.invoice_id = i.id) WHERE i.categ = ’%s_invoice’ GROUP BY i.partner_id ””” self._cr.execute(query % categ) return self._cr.fetchall() Better, but it could still be called indirectly! Now private!

28. Where should we store precious tokens for external APIs? On the res.users record of the user! On the res.company record! On the record representing the API endpoint, like the acquirer record! In the ir.config_parameter table! Wherever it makes the most sense , as long as it’s not readable by anyone! ( field-level group , ACLs , ICP groups , etc.)

5. There are safer and smarter ways to parse data in PYTHON “ 42” int (x) float (x) Parse it like this “ [1,2,3, true ]” json .loads(x) ‘ { “ widget ” : “ monetary ” }’ “ [1,2,3, True ]” ast. literal_eval(x) ” {‘widget’: ‘monetary’}” Given this string

20. This method is safe against SQL injection def _compute_balance_by_category ( self , categ = ’in’ ): categ = ’%s_invoice’ % categ query = ”””SELECT sum(debit-credit) FROM account_invoice_line l JOIN account_invoice i ON (l.invoice_id = i.id) WHERE i.categ = %s GROUP BY i.partner_id ””” self._cr.execute(query, (categ,) ) return self._cr.fetchall() Separates code and parameters !

7. If you must eval parameters use a safe eval method # YES from odoo.tools import safe_eval res = safe_eval( ’foo’ , { ’foo’ : 42 }); # NO from odoo.tools import safe_eval as eval res = eval( ’foo’ , { ’foo’ : 42 }); P Y T H O N Alias built-in eval as ” unsafe_eval ” Show your meaning! # YES unsafe_eval = eval res = unsafe_eval(trusted_code); # NO! res = eval(trusted_code); Import as ” safe_eval ”, not as ”eval”!

39. Odoo ACLs are not always trivial  user  group B  group A  data  C  R  U  D Group Access Rule (ir.rule) Global Access Rule (ir.rule)    1 2 3 Per-model Per-record A C C E S S G R A N T E D Group Access Rights (ir.model.access)

37. Odoo ACLs are not always trivial  user  group B  group A  data  C  R  U  D  C  R  U  D Group Access Rule (ir.rule) Global Access Rule (ir.rule)    1 2 3 Per-model Per-record A C C E S S G R A N T E D Group Access Rights (ir.model.access)

38. Odoo ACLs are not always trivial  user  group B  group A  data  C  R  U  D  C  R  U  D Group Access Rule (ir.rule) Global Access Rule (ir.rule)    1 2 3 Per-model Per-record  C  R  U  D A C C E S S D E N I E D Group Access Rights (ir.model.access)

35. Odoo ACLs are not always trivial  user  group B  group A  data  C  R  U  D  C  R  U  D  C  R  U  D Group Access Rights (ir.model.access) Group Access Rule (ir.rule) Global Access Rule (ir.rule)    1 2 3 Per-model Per-record A C C E S S D E N I E D

36. Odoo ACLs are not always trivial  user  group B  group A  data  C  R  U  D  C  R  U  D  C  R  U  D Group Access Rule (ir.rule) Global Access Rule (ir.rule)    1 2 3 Per-model Per-record A C C E S S G R A N T E D Group Access Rights (ir.model.access)

Views

  • 20 Total Views
  • 0 Website Views
  • 20 Embeded Views

Actions

  • 0 Social Shares
  • 6 Likes
  • 0 Dislikes
  • 0 Comments

Share count

  • 0 Facebook
  • 0 Twitter
  • 0 LinkedIn
  • 0 Google+

Embeds 3

  • 4 accounts.odoo.com
  • 11 ec2-46-137-189-158.eu-west-1.compute.amazonaws.com:9069
  • 3 onlinesync.odoo.com