Univention Bugzilla – Bug 33684
syntax.IComputer_FQDN parsing of long value ends in endless loop
Last modified: 2023-03-15 14:14:28 CET
takes 3 seconds for e.g. "aaaaaaaaaaaaaaaaaaaa.b.c" takes ages for a str with length 70: univention.admin.syntax.IComputer_FQDN.parse('%s.b.c' % ('a' * 64)) Also for the derived classes DomainController, Windows_Server, UCS_Server, ServicePrint_FQDN.
This is the currente regular expression: > regex = re.compile( '(?=^.{1,254}$)(^(?:(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z0-9]{2,})$)' ) It is very slow even for 25 characters long hostname. > In [25]: time regex.match('%s.b.c' % ('a' * 25)) > CPU times: user 14.95 s, sys: 0.00 s, total: 14.95 s > Wall time: 15.39 s 22 characters: > In [29]: time regex.match('%s.b.c' % ('a' * 22)) > CPU times: user 1.82 s, sys: 0.00 s, total: 1.82 s > Wall time: 1.87 s @Ingo: Could that lead to problems in larger projects?
We had the same problems before in system-setup and installer, the solution was to split on '.' and apply regex on the pieces. Bug #24888 Bug #28574
Nice DoS if there are places where users can enter values.
This issue has been filed against UCS 4.2. UCS 4.2 is out of maintenance and many UCS components have changed in later releases. Thus, this issue is now being closed. If this issue still occurs in newer UCS versions, please use "Clone this bug" or reopen it and update the UCS version. In this case please provide detailed information on how this issue is affecting you.
(In reply to Alexander Kläser from comment #1) > This is the current regular expression: > > > r'(?=^.{1,254}$)(^(?:(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z0-9]{2,})$)' For readability as `re.VERBOSE`: r""" (?=^.{1,254}$) # whole string length 1…254 (^ (?: (?!\d+\.) [a-zA-Z0-9_-]{1,63} \.?)+ # label length 1…63 but not all digits # ^
Again as Bugzilla does not allow Unicode: (In reply to Alexander Kläser from comment #1) > This is the current regular expression: > > > r'(?=^.{1,254}$)(^(?:(?!\d+\.)[a-zA-Z0-9_\-]{1,63}\.?)+(?:[a-zA-Z0-9]{2,})$)' For readability as `re.VERBOSE`: r""" (?=^.{1,254}$) # whole string length 1…254 (^ (?: (?!\d+\.) [a-zA-Z0-9_-]{1,63} \.?)+ # label length 1…63 but not all digits # ^ (?:[a-zA-Z0-9]{2,}) # TLD # ^ $) """ The slowness is primarily caused by `?` and to a lesser degree to the `{2,}` as they both lead to back-tracking. Also missing is the constraint to not start and end on dash compared to `hostName`: > r"^ (?![_-]) [a-zA-Z0-9_-]{1,63} (?<![_-]) $" `dnsHostname: > r'^ (?![0-9]+$ | [_-]) [a-zA-Z0-9_-]{1,63} (?<![_-]) $' https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
Package: univention-directory-manager-modules Version: 15.0.20-2A~5.0.0.202302171803 Branch: ucs_5.0-0 Scope: errata5.0-3 [5.0-3] 703d5654ef Bug #33684: univention-directory-manager-modules 15.0.20-2A~5.0.0.202302171803 doc/errata/staging/univention-directory-manager-modules.yaml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) [5.0-3] 6efa8337c0 fix(udm): slow and invalid syntax for IComputer_FQDN doc/errata/staging/univention-directory-manager-modules.yaml | 12 +++++++++ .../univention-directory-manager-modules/debian/changelog | 6 +++++ .../modules/univention/admin/syntax.py | 38 ++++++++++++++++++++++++++++- 3 files changed, 55 insertions(+), 1 deletion(-)
Unfortunate this breaks tests: https://univention-dist-jenkins.k8s.knut.univention.de/job/UCS-5.0/job/UCS-5.0-3/job/AutotestJoin/SambaVersion=no-samba,Systemrolle=master/lastCompletedBuild/testReport/10_ldap/44replication_binary/master090/ → E: Invalid syntax: Mail home server: Not a valid FQDN. --set mailHomeServer=master090 https://univention-dist-jenkins.k8s.knut.univention.de/job/UCS-5.0/job/UCS-5.0-3/job/AutotestJoin/SambaVersion=no-samba,Systemrolle=master/lastCompletedBuild/testReport/59_udm/71_test_udm_settings_usertemplate/test_use_usertemplate_umlauts/ → E: Invalid syntax: Host: Not a valid FQDN. shares/share kwargs={'name': 'uj307r7s8m', 'path': '/edmehfjs1c', 'host': 'h8mc0eblnr'} … some other shares/share tests.
(In reply to Florian Best from comment #8) > Unfortunate this breaks tests: as the tests are all wrong: the given "host-names" are not *fully qualified* and as such invalid. Even before my change > key = '%(name)s.%(domain)s' was set, so only FQHNs should be allowed: - UDM-GUI allowed you to only select FQHNs - but UDM-CLI accepted wrong values until now Therefor I'm changing the tests to set a correct FQHN. Affected classes are: - IComputer_FQDN - DomainController - policies/ldapserver: ldapServer - Windows_Server - UCS_Server - appcenter/app: server - policies/repositoryserver: repositoryServer - shares/share: host - ServicePrint_FQDN - policies/printserver: printServer - shares/print: spoolHost - shares/printer: spoolHost - shares/printergroup: spoolHost - MailHomeServer - mail/folder: mailHomeServer - users/user: mailHomeServer
[5.0-3] c5f44ca660 test(udm): Fix tests to use correct FQHN test/ucs-test/debian/changelog | 6 ++ test/ucs-test/tests/01_base/16_policy-update.py | 49 +++++++++++ .../tests/01_base/16_test_univention_base_files_scripts.py | 132 ---------------------------- test/ucs-test/tests/01_base/17_nfsmounts.py | 112 +++++++++++++++++++++++ test/ucs-test/tests/10_ldap/44replication_binary | 19 ++-- .../59_udm/14_remove_values_from_share_sambaCustomSettings.py | 2 +- test/ucs-test/tests/59_udm/61_test_udm_users_unittests.py | 21 ++--- test/ucs-test/tests/59_udm/71_test_udm_settings_usertemplate.py | 4 +- 8 files changed, 191 insertions(+), 154 deletions(-) [5.0-3] 21f3fc8aef fix(ucs-test): 01/17_nfsmounts test/ucs-test/debian/changelog | 6 ++++++ 1 file changed, 6 insertions(+) [5.0-3] dec4c32bee test(udm): Fix 01/17_nfsmounts test/ucs-test/tests/01_base/17_nfsmounts.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) Package: ucs-test Version: 10.0.10-8A~5.0.0.202302222030 Branch: ucs_5.0-0 Scope: errata5.0-3
Jenkins tests are mostly fixed except one: https://univention-dist-jenkins.k8s.knut.univention.de/job/UCS-5.0/job/UCS-5.0-3/job/AutotestJoin/lastCompletedBuild/SambaVersion=s4,Systemrolle=master/testReport/59_udm/71_test_udm_settings_usertemplate/test_use_usertemplate_umlauts/
(In reply to Philipp Hahn from comment #11) > Jenkins tests are mostly fixed except one: > 59_udm/71_test_udm_settings_usertemplate/test_use_usertemplate_umlauts The code has already been fixed. Running the test again locally showed it as fixed. No idea why it failed in Jenkins last night.
(In reply to Philipp Hahn from comment #12) > (In reply to Philipp Hahn from comment #11) > > Jenkins tests are mostly fixed except one: > > 59_udm/71_test_udm_settings_usertemplate/test_use_usertemplate_umlauts > > The code has already been fixed. > Running the test again locally showed it as fixed. > No idea why it failed in Jenkins last night. Because I missed one case: https://git.knut.univention.de/univention/ucs/-/merge_requests/678/diffs?commit_id=1218e71b8ad58c16d43d2da71a83e9444985ac5e
[5.0-3] edaf5b9011 test(59/71): Fix slow and invalid syntax for IComputer_FQDN test/ucs-test/debian/changelog | 8 ++++++++ test/ucs-test/tests/59_udm/71_test_udm_settings_usertemplate.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) Package: ucs-test Version: 10.0.10-10A~5.0.0.202302231134 Branch: ucs_5.0-0 Scope: errata5.0-3
OK: new regex OK: test adjustments - mentioned syntax classes now only support a fqdn OK: YAML
<https://errata.software-univention.de/#/?erratum=5.0x614>