0001175: bareos crashes with invalid character in ACL

ID	Project	Category	View Status	Date Submitted	Last Update

0001175	bareos-core	director	public	2020-02-06 11:35	2020-11-30 15:46

Reporter	Gabscap	Assigned To	arogge
Priority	high	Severity	crash	Reproducibility	always
Status	closed	Resolution	fixed
Platform	Linux	OS	Debian	OS Version	10
Product Version	19.2.5

Summary	0001175: bareos crashes with invalid character in ACL
Description	Bareos detects invalid character in the ACL correctly, but instead of canceling the configuration-loading it aborts with an exception. --- bareos-dbcheck crashes with: realloc(): invalid pointer Aborted gdb revealed that the crash happens in this line because of an invalid character '(': https://github.com/bareos/bareos/blob/b911f9bb4bb0013fccd60d03f20134406f60f301/core/src/lib/edit.cc#L614 According to the config, ACLs may use regular expressions. So please allow following symbols: ()[]*+\ This bug leads to the BackupCatalog job failing with above error message.
Steps To Reproduce	Insert a regex special char (e.g. '(') into the WhereACL Option in any console/*.conf file. $ /usr/sbin/bareos-dbcheck realloc(): invalid pointer Aborted
Tags	crash

Gabscap 2020-02-06 11:53 reporter ~0003729 Last edited: 2020-02-06 11:56	Typo: s/According to the config/According to the documentation/ And i forgot to mention the regex special chars: {}

arogge 2020-02-06 12:12 manager ~0003730	Hi, thanks for taking the time to report this problem. I tried to reproduce it and noticed that the director will crash with such a configuration which is even worse.

arogge 2020-02-06 12:18 manager ~0003731	Not taking into account whether or not a character is valid for the ACL, the software should not terminate when a character is invalid.

arogge 2020-02-06 15:24 manager ~0003733	I have created a PR for this: https://github.com/bareos/bareos/pull/410 That PR does not yet add the missing characters in the ACL check, but it will make sure neither the director nor dbcheck will crash when an invalid ACL string is detected. Any chance you can give the packages a try once they're ready?

Gabscap 2020-02-06 18:19 reporter ~0003739	Yes, I can upgrade when the packages are available.

arogge 2020-02-07 09:02 manager ~0003744	The packages can be installed from https://download.bareos.org/bareos/experimental/CD/PR-410/ This will mitigate the crash, but not (yet) allow the additional characters.

Gabscap 2020-02-07 16:51 reporter ~0003747	bareos-dbcheck works now.

arogge 2020-02-07 16:57 manager ~0003748	Glad to hear that. Did the extended regexp (with the parentheses and braces) in the ACL you had configured actually work in 18.2? We will probably consider correcting the set of valid ACL characters then.

Gabscap 2020-02-07 17:03 reporter ~0003749	Yes, I used character classes [A-Za-z0-9_-], +, ? and () for grouping. All of them worked fine. I didn't use {}, so I can't comment on that.

arogge 2020-02-07 17:13 manager ~0003751	So I guess it would be nice to allow: "[]()+?" for your use-case and also "*\|" for other cases. We probably won't allow "{}," as commas (,) were what made us implement the validity check in the first place: Some users ran into issues, because they declared an ACL like "name1,name2,name3" instead of "name1", "name2", "name3" and the parser wasn't able to find that issue.

Gabscap 2020-02-07 17:24 reporter ~0003752 Last edited: 2020-02-07 17:24	Sounds good. Thank You for resolving this issue.

arogge 2020-02-10 13:51 manager ~0003763	I have amended the PR to allow the additional characters. I'll inform you as soon as a new version is available on the dl-mirror, so you can test again.

arogge 2020-02-10 16:12 manager ~0003764	Would you check that the new packages allow to use your old ACLs again? Thank you!

arogge 2020-02-11 14:22 manager ~0003779	Fix committed to bareos bareos-19.2 branch with changesetid 12849.

arogge 2020-02-11 17:19 manager ~0003787	Fixed in Bareos 19.2.6

bareos: master 8d92e8f7 2020-02-06 13:36 arogge Ported: N/A Details Diff	systemtests: check crash on invalid ACL Bug 0001175 - bareos crashes with invalid character in ACL When a profile configuration contains an entry that is detected as invalid, the director crashes. This test reproduces this problem.	Affected Issues 0001175
	mod - systemtests/CMakeLists.txt	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/catalog/MyCatalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/console/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/fileset/Catalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/fileset/SelfTest.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/BackupCatalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/RestoreFiles.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/jobdefs/DefaultJob.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/messages/Daemon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/messages/Standard.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Differential.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Full.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Incremental.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Scratch.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/profile/operator.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/storage/File.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/client/myself.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/director/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/messages/Standard.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/device/FileStorage.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/director/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/messages/Standard.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/storage/bareos-sd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bconsole.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/client/FileDaemon-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/director/Director-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/monitor/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/storage/StorageDaemon-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/testrunner	Diff File
bareos: master d3983ba1 2020-02-06 16:03 arogge Ported: N/A Details Diff	lib: make IsAclEntryValid() more robust Fixes 0001175: bareos crashes with invalid character in ACL Previously IsAclEntryValid() took a char* as a parameter that was expected to be a POOLMEM (and would have been resized when an error occured). This char* has been replaced by a std::vector<char>& and the new overload for Mmsg() is now used to format a message.	Affected Issues 0001175
	mod - core/src/dird/dird_conf.cc	Diff File
	mod - core/src/lib/edit.cc	Diff File
	mod - core/src/lib/edit.h	Diff File
	mod - core/src/tests/test_acl_entry_syntax.cc	Diff File
bareos: bareos-19.2 208119f1 2020-02-06 16:03 arogge Ported: N/A Details Diff	lib: make IsAclEntryValid() more robust Fixes 0001175: bareos crashes with invalid character in ACL Previously IsAclEntryValid() took a char* as a parameter that was expected to be a POOLMEM (and would have been resized when an error occured). This char* has been replaced by a std::vector<char>& and the new overload for Mmsg() is now used to format a message. (cherry picked from commit d3983ba1d08d9bda69ed58c36ec4253353106344)	Affected Issues 0001175
	mod - core/src/dird/dird_conf.cc	Diff File
	mod - core/src/lib/edit.cc	Diff File
	mod - core/src/lib/edit.h	Diff File
	mod - core/src/tests/test_acl_entry_syntax.cc	Diff File
bareos: master 1449105d 2020-02-10 14:34 arogge Ported: N/A Details Diff	lib: allow more chars in IsAclEntryValid() Bug 0001175: bareos crashes with invalid character in ACL The newly introduced IsAclEntryValid() allowed only "!.:_-'/" as chars in an ACL. As regular expressions are possible in ACLs this list did not allow some previously valid ACLs to be configured. This patch extends the list to allow "!()[]\|+?.:_-'/" which will allow most syntaxes. We do not add "{}," intentionally, because this would re-allow the wrong ACL syntaxes that the original change tried to catch (i.e. adding a string with comma-separated resource names instead of a list of resource names).	Affected Issues 0001175
	mod - core/src/lib/edit.cc	Diff File
bareos: bareos-19.2 07e70fee 2020-02-10 14:34 arogge Ported: N/A Details Diff	lib: allow more chars in IsAclEntryValid() Bug 0001175: bareos crashes with invalid character in ACL The newly introduced IsAclEntryValid() allowed only "!.:_-'/" as chars in an ACL. As regular expressions are possible in ACLs this list did not allow some previously valid ACLs to be configured. This patch extends the list to allow "!()[]\|+?.:_-'/" which will allow most syntaxes. We do not add "{}," intentionally, because this would re-allow the wrong ACL syntaxes that the original change tried to catch (i.e. adding a string with comma-separated resource names instead of a list of resource names). (cherry picked from commit 1449105d157f8a713aa508240ee487ea1c4cf282)	Affected Issues 0001175
	mod - core/src/lib/edit.cc	Diff File
bareos: master 3b6ebeac 2020-02-11 14:30 arogge Committer: GitHub Ported: N/A Details Diff	Merge pull request 0000410 from arogge/dev/arogge/master/fix-1175 Fix Bug 0001175: bareos crashes with invalid character in ACL	Affected Issues 0001175
	mod - core/src/dird/dird_conf.cc	Diff File
	mod - core/src/include/baconfig.h	Diff File
	mod - core/src/lib/edit.cc	Diff File
	mod - core/src/lib/edit.h	Diff File
	mod - core/src/lib/message.cc	Diff File
	mod - core/src/lib/message.h	Diff File
	mod - core/src/tests/test_acl_entry_syntax.cc	Diff File
	mod - systemtests/CMakeLists.txt	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/catalog/MyCatalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/client/bareos-fd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/console/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/fileset/Catalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/fileset/SelfTest.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/BackupCatalog.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/RestoreFiles.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/job/backup-bareos-fd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/jobdefs/DefaultJob.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/messages/Daemon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/messages/Standard.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Differential.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Full.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Incremental.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/pool/Scratch.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/profile/operator.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-dir.d/storage/File.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/client/myself.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/director/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-fd.d/messages/Standard.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/device/FileStorage.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/director/bareos-dir.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/director/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/messages/Standard.conf	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bareos-sd.d/storage/bareos-sd.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/bconsole.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/client/FileDaemon-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/director/Director-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/monitor/bareos-mon.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/etc/bareos/tray-monitor.d/storage/StorageDaemon-local.conf.in	Diff File
	add - systemtests/tests/config-syntax-crash/testrunner	Diff File

Date Modified	Username	Field	Change
2020-02-06 11:35	Gabscap	New Issue
2020-02-06 11:35	Gabscap	Tag Attached: crash
2020-02-06 11:53	Gabscap	Note Added: 0003729
2020-02-06 11:56	Gabscap	Note Edited: 0003729
2020-02-06 12:12	arogge	Assigned To	=> arogge
2020-02-06 12:12	arogge	Status	new => confirmed
2020-02-06 12:12	arogge	Note Added: 0003730
2020-02-06 12:13	arogge	Severity	major => crash
2020-02-06 12:13	arogge	Product Version	19.2.4 => 19.2.5
2020-02-06 12:18	arogge	Note Added: 0003731
2020-02-06 12:34	arogge	Summary	bareos-dbcheck crashes => bareos crashes with invalid character in ACL
2020-02-06 12:34	arogge	Description Updated
2020-02-06 12:35	arogge	Status	confirmed => assigned
2020-02-06 15:24	arogge	Note Added: 0003733
2020-02-06 18:19	Gabscap	Note Added: 0003739
2020-02-07 09:02	arogge	Note Added: 0003744
2020-02-07 16:51	Gabscap	Note Added: 0003747
2020-02-07 16:57	arogge	Note Added: 0003748
2020-02-07 17:03	Gabscap	Note Added: 0003749
2020-02-07 17:13	arogge	Note Added: 0003751
2020-02-07 17:24	Gabscap	Note Added: 0003752
2020-02-07 17:24	Gabscap	Note Edited: 0003752
2020-02-10 10:54	arogge	Relationship added	related to 0001177
2020-02-10 13:51	arogge	Note Added: 0003763
2020-02-10 16:12	arogge	Status	assigned => feedback
2020-02-10 16:12	arogge	Note Added: 0003764
2020-02-11 14:22	arogge	Changeset attached	=> bareos master 3b6ebeac
2020-02-11 14:22	arogge	Changeset attached	=> bareos master 1449105d
2020-02-11 14:22	arogge	Changeset attached	=> bareos master d3983ba1
2020-02-11 14:22	arogge	Changeset attached	=> bareos master 8d92e8f7
2020-02-11 14:22	arogge	Changeset attached	=> bareos bareos-19.2 07e70fee
2020-02-11 14:22	arogge	Changeset attached	=> bareos bareos-19.2 208119f1
2020-02-11 14:22	arogge	Note Added: 0003779
2020-02-11 14:22	arogge	Status	feedback => resolved
2020-02-11 14:22	arogge	Resolution	open => fixed
2020-02-11 17:19	arogge	Status	resolved => closed
2020-02-11 17:19	arogge	Note Added: 0003787
2020-11-30 15:46	arogge	Relationship added	related to 0001282

Reporting new Issues is disabled, please Report new Issues at https://github.com/bareos/bareos/issues

View Issue Details

Relationships

Activities

Related Changesets

Issue History

related to	0001177	closed	arogge	Release Bareos 19.2.6
related to	0001282	closed	arogge	Release Bareos 20.0.0