View Issue Details

IDProjectCategoryView StatusLast Update
0001175bareos-core[All Projects] directorpublic2020-02-11 17:19
ReporterGabscapAssigned Toarogge 
PriorityhighSeveritycrashReproducibilityalways
Status closedResolutionfixed 
PlatformLinuxOSDebianOS Version10
Product Version19.2.5 
Fixed in Version 
Summary0001175: bareos crashes with invalid character in ACL
DescriptionBareos 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 ReproduceInsert a regex special char (e.g. '(') into the WhereACL Option in any console/*.conf file.

$ /usr/sbin/bareos-dbcheck
realloc(): invalid pointer
Aborted
Tagscrash
bareos-master: impactyes
bareos-master: actionfixed
bareos-19.2: impactyes
bareos-19.2: actionfixed
bareos-18.2: impactno
bareos-18.2: action
bareos-17.2: impactno
bareos-17.2: action
bareos-16.2: impactno
bareos-16.2: action
bareos-15.2: impact
bareos-15.2: action
bareos-14.2: impact
bareos-14.2: action
bareos-13.2: impact
bareos-13.2: action
bareos-12.4: impact
bareos-12.4: action

Relationships

related to 0001177 closedarogge Release Bareos 19.2.6 

Activities

Gabscap

Gabscap

2020-02-06 11:53

reporter   ~0003729

Last edited: 2020-02-06 11:56

View 2 revisions

Typo:
s/According to the config/According to the documentation/

And i forgot to mention the regex special chars: {}

arogge

arogge

2020-02-06 12:12

developer   ~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

arogge

2020-02-06 12:18

developer   ~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

arogge

2020-02-06 15:24

developer   ~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

Gabscap

2020-02-06 18:19

reporter   ~0003739

Yes, I can upgrade when the packages are available.
arogge

arogge

2020-02-07 09:02

developer   ~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

Gabscap

2020-02-07 16:51

reporter   ~0003747

bareos-dbcheck works now.
arogge

arogge

2020-02-07 16:57

developer   ~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

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

arogge

2020-02-07 17:13

developer   ~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

Gabscap

2020-02-07 17:24

reporter   ~0003752

Last edited: 2020-02-07 17:24

View 2 revisions

Sounds good. Thank You for resolving this issue.

arogge

arogge

2020-02-10 13:51

developer   ~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

arogge

2020-02-10 16:12

developer   ~0003764

Would you check that the new packages allow to use your old ACLs again?
Thank you!
arogge

arogge

2020-02-11 14:22

developer   ~0003779

Fix committed to bareos bareos-19.2 branch with changesetid 12849.
arogge

arogge

2020-02-11 17:19

developer   ~0003787

Fixed in Bareos 19.2.6

Related Changesets

bareos: master 8d92e8f7

2020-02-06 12:36:14

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 15:03:57

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 15:03:57

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 13:34:43

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 13:34:43

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 13:30:31

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

Issue History

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 View Revisions
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:13 arogge bareos-master: impact => yes
2020-02-06 12:13 arogge bareos-master: action => will care
2020-02-06 12:13 arogge bareos-19.2: impact => yes
2020-02-06 12:13 arogge bareos-19.2: action => will care
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 View Revisions
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 View Revisions
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 bareos-master: action will care => fixed
2020-02-11 17:19 arogge bareos-19.2: action will care => fixed
2020-02-11 17:19 arogge bareos-18.2: impact => no
2020-02-11 17:19 arogge bareos-17.2: impact => no
2020-02-11 17:19 arogge bareos-16.2: impact => no
2020-02-11 17:19 arogge Note Added: 0003787