[9.0][FIX] Mail tracking bugfixes

[antespi]

This PR fix two bugs found in mail_tracking:

• When an email notification is an answer of another email notification (for example, in a invoice mail thread), then mail_tracking addon add several tracking email IDs in body. This fix assure that there is only one ID (and the last one).
• Support for multi-company. Mail tracking status of partners from other companies raise an access error exception. This fix get status as sudo.

[yajo]

I find all of this would be much easier to maintain and understand by using lxml.html to parse the email body, and then XPath to find/remove the node. This regexp for instance would produce false positives with i.e. <img src="i'm a hacker>"/>, and given those come from the outside world, it's a good attack vector.

[antespi]

Yes, this is another way to do it.
I think regex is faster and consume less memory. BTW, that vector is not matching the regex, see https://regex101.com/r/lW4cB1/3

[yajo]

Indeed, I was wrong, but see:

• <img data-odoo-tracking-email="0'/>
• <imgdata-odoo-tracking-email="0'/>
• <img <broken syntax!! data-odoo-tracking-email="0'/>
• <img <broken syntax!! data-odoo-tracking-email="0' muahahahaa>
• <img valid-tag=">" data-odoo-tracking-email="1999"/>

want more? 😏

[antespi]

Well, in those cases lxml will break, isn't it?

This fix first remove any tag that match with this regex, then inject the tag and after read the tag, so if there is some broken tag then it's not a security problem.

Using lxml, what would the code have to do if no valid HTML is read?

[yajo]

I think it should except and pass in such case, don't you think?

[antespi]

I think that using lxml and then except and pass when invalid tags in the email will miss a tracking for some emails. In my approach (btw, the current one) those corner cases will be tracked without any problem.

So, I prefer to keep the current implementation because:

• It's more resilience
• It has a better CPU & RAM performance
• It's simpler
• It's the current implementation, less code changes
• It hasn't any security issue

Any other opinions here?

[rafaelbn]

Thanks! 👍

[pedrobaeza]

Please change module version number. Does this apply to v10? If so, please forward port it.

[yajo]

Done for him, no problem on final squashing, could you review @pedrobaeza?

[pedrobaeza]

Merged squashing