Thank you, both points addressed. ** Description changed:
SRU justification: [ Impact ] * ods-signerd fails to start, thus rendering an OpenDNSSEC installation useless. * The issue is caused by __snprintf_chk aborting after snprintf() is called with a fixed buffer size argument and a buffer that is calloc()ed with a dynamically-computed size. There is no buffer overflow (the dynamically-computed buffer size is correct), thus this issue is only observable when compiled with _FORTIFY_SOURCE. * Upstream have fixed this issue with two commits, but no new version has yet been released: - https://github.com/opendnssec/opendnssec/commit/02940f512de662152d331df5afb8ad3436f96224 - - https://github.com/opendnssec/opendnssec/pull/869/changes/170f507ccb1a955460a78e59a0382b0aab4889fe + - https://github.com/opendnssec/opendnssec/commit/b7b69f7090e0180354a342bc54449e065987f3f6 * ods-signerd on jammy and older work well, despite the problematic code being present. I'm happy to open MPs for jammy and older, too, if this is desired. * Original bug description kept below. [ Test Plan ] 1. Install opendnssec-signer and softhsm2 - $ sudo apt install opendnssec-signer softhsm2 + $ sudo aptitude install opendnssec-signer softhsm2 - 2. Give opendnssec user access to softhsm2 tokens directory + 2. Given opendnssec user access to softhsm2 tokens directory (/var/lib/softhsm/tokens/) via group membership. $ sudo adduser opendnssec softhsm - 3. Initialise a new softhsm2 token and specify ‘1234’ as a PIN (as per the configuration file below) + 3. Initialise a new softhsm2 token $ sudo runuser -u opendnssec -- softhsm2-util --init-token --slot 0 --label 'OpenDNSSEC' 4. Create a test directory $ mkdir /tmp/ods/ 5. Create a minimal configuration file $ cat >/tmp/ods/conf.xml <<EOF <?xml version="1.0" encoding="UTF-8"?> <Configuration> <RepositoryList> <Repository name="SoftHSM"> <Module>/usr/lib/softhsm/libsofthsm2.so</Module> <TokenLabel>OpenDNSSEC</TokenLabel> <PIN>1234</PIN> </Repository> </RepositoryList> <Common> <PolicyFile>/dev/null</PolicyFile> <ZoneListFile>/dev/null</ZoneListFile> </Common> <Enforcer> <Datastore><SQLite>/dev/null</SQLite></Datastore> <WorkingDirectory>/tmp/ods/</WorkingDirectory> </Enforcer> <Signer> <WorkingDirectory>/tmp/ods/</WorkingDirectory> <WorkerThreads>4</WorkerThreads> <NotifyCommand>/usr/bin/echo %zone</NotifyCommand> </Signer> </Configuration> EOF 6. Create a minimal zone list $ cat >/tmp/ods/zones.xml <<EOF <?xml version="1.0" encoding="UTF-8"?> <ZoneList> <Zone name="example.com"> <Policy>default</Policy> <SignerConfiguration>/dev/null</SignerConfiguration> <Adapters> <Input> <Adapter type="File">/dev/null</Adapter> </Input> <Output> <Adapter type="File">/dev/null</Adapter> </Output> </Adapters> </Zone> </ZoneList> EOF 7. Please note that this is deliberately a very short example, hence the /dev/null files. The zones.xml filename is important, hard-coded and relative - to the WorkingDirectory configuration option. It is normally generated by the ods-signer admin tool, but is manually constructed here for simplicity. + to the WorkingDirectory configuration option. 8. Run the signer daemon in the foreground (-d) and passing the configuration file $ sudo runuser -u opendnssec -- ods-signerd -d -c /tmp/ods/conf.xml 9. Observe the abort. [ Where problems could occur ] * The previous code was considerably more convoluted than the new one, which uses strncat and strlcpy. The function in question performs a simple string search and replacement. * There are three inputs to the function: - The original string, from the configuration file ("/usr/bin/echo %zone" in the test). This is expected to be administrator-controlled. - The placeholder, hardcoded in code ("%zone"). - The replacement, which could be either administrator-controlled or untrusted input (the zone name, "example.com"). * If the code is incorrect and the third argument is untrusted input, it could lead to a buffer overflow. Heap protection compiler flags should help reduce the risk of code execution and result in process crashes, instead. * I have omitted the patch hunks which referenced some testing files that do not exist in the current Ubuntu sources. The test file included is not used, but I kept it to reduce the likelihood that future patches do not apply cleanly. [ Other Info ] * I built the package here: - https://launchpad.net/~lucistanescu/+archive/ubuntu/testing/+packages Patch is identical, apart from the version in debian/changelog which has a '~noble1' suffix. Original bug description: ods-signerd does not start but crashes with buffer overflow. Linux outpost 6.8.0-49-generic #49-Ubuntu SMP PREEMPT_DYNAMIC Mon Nov 4 02:06:24 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux *** buffer overflow detected ***: terminated Aborted: unknown pthread_kill gsignal abort unknown unknown unknown __snprintf_chk ods_replace unknown unknown engine_start main Threaddump: unknown __select pselect netio_dispatch xfrhandler_start unknown unknown unknown Threaddump: unknown __select pselect netio_dispatch dnshandler_start unknown unknown unknown Threaddump: unknown __select cmdhandler_start unknown unknown unknown -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2089834 Title: ods-signerd crashes at start after upgrade 22.04 to 24.04 LTS To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/opendnssec/+bug/2089834/+subscriptions -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
