From 97c0a110e87d3032270fc0488c6740faafca81b7 Mon Sep 17 00:00:00 2001
From: Paul Morelle <pmo@odoo.com>
Date: Thu, 10 Dec 2020 11:41:17 +0000
Subject: [PATCH] [FIX] account: fix regexp when partner name contains +
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When the name of a partner contains a regular expression operator, such
as grouping parenthesis, +, *, brackets... the resulting regexp may
match wrong names or worse be invalid and lead to an exception.

With this commit, we create a regular expression from only the words
formed from at least 3 of [a-zA-Z0-9], ignoring all other characters,
including those who may have a meaning in a regular expression.

This avoids these issues:
- regexp injection (security issue)
- punctuation resulting in erroneous regular expression ("ABC + SPRL"
  resulted in "(?=.*ABC.*)(?=.*+.*)(?=.*SPRL.*)" which is invalid at
  the ".*+" part)
- false positives due to words that are too small (\b were added, and
  the minimal matching word size is 3)
- accentuation mismatch (both sides are filtered through unaccent)
- special cases in unicode that have a special behavior with lower().

See the discussion on odoo/odoo#63145 for a detailled explaination about
these issues.

OPW-2360687

closes odoo/odoo#64510

X-original-commit: 3c5296863f0030acc5d3dba0ff73a6f1b42d1255
Signed-off-by: William André (wan) <wan@odoo.com>
Signed-off-by: Paul Morelle <madprog@users.noreply.github.com>
---
 addons/account/models/account_reconcile_model.py     |  8 +++++++-
 .../tests/test_reconciliation_matching_rules.py      | 12 ++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/addons/account/models/account_reconcile_model.py b/addons/account/models/account_reconcile_model.py
index b22aa22a9e9..757702a1612 100644
--- a/addons/account/models/account_reconcile_model.py
+++ b/addons/account/models/account_reconcile_model.py
@@ -2,6 +2,7 @@
 
 from odoo import api, fields, models, _
 from odoo.tools import float_compare, float_is_zero
+from odoo.osv.expression import get_unaccent_wrapper
 from odoo.exceptions import UserError, ValidationError
 import re
 from math import copysign
@@ -582,6 +583,8 @@ class AccountReconcileModel(models.Model):
         if self.rule_type != 'invoice_matching':
             raise UserError(_('Programmation Error: Can\'t call _get_invoice_matching_query() for different rules than \'invoice_matching\''))
 
+        unaccent = get_unaccent_wrapper(self._cr)
+
         # N.B: 'communication_flag' is there to distinguish invoice matching through the number/reference
         # (higher priority) from invoice matching using the partner (lower priority).
         query = r'''
@@ -641,7 +644,10 @@ class AccountReconcileModel(models.Model):
                         within the payment_ref, in any order, with any characters between them. */
 
                         aml_partner.name IS NOT NULL
-                        AND st_line.payment_ref ~* concat('(?=.*', array_to_string(regexp_split_to_array(lower(aml_partner.name), ' '),'.*)(?=.*'), '.*)')
+                        AND """ + unaccent("st_line.payment_ref") + r""" ~* ('^' || (
+                            SELECT string_agg(concat('(?=.*\m', chunk[1], '\M)'), '')
+                              FROM regexp_matches(""" + unaccent("aml_partner.name") + r""", '\w{3,}', 'g') AS chunk
+                        ))
                     )
                 """
 
diff --git a/addons/account/tests/test_reconciliation_matching_rules.py b/addons/account/tests/test_reconciliation_matching_rules.py
index b31e40eab5c..cc9e9d73d7c 100644
--- a/addons/account/tests/test_reconciliation_matching_rules.py
+++ b/addons/account/tests/test_reconciliation_matching_rules.py
@@ -678,6 +678,18 @@ class TestReconciliationMatchingRules(AccountTestInvoicingCommon):
             self.bank_line_2.id: {'aml_ids': []},
         }, self.bank_st)
 
+    def test_partner_name_with_regexp_chars(self):
+        self.invoice_line_1.partner_id.write({'name': "Archibald + Haddock"})
+        self.bank_line_1.write({'partner_id': None, 'payment_ref': '1234//HADDOCK+Archibald'})
+        self.bank_line_2.write({'partner_id': None})
+        self.rule_1.write({'match_partner': False})
+
+        # The query should still work
+        self._check_statement_matching(self.rule_1, {
+            self.bank_line_1.id: {'aml_ids': [self.invoice_line_1.id], 'model': self.rule_1, 'partner': self.bank_line_1.partner_id},
+            self.bank_line_2.id: {'aml_ids': []},
+        }, self.bank_st)
+
     def test_match_multi_currencies(self):
         ''' Ensure the matching of candidates is made using the right statement line currency.