Page MenuHome GnuPG
Create Task
Maniphest T5279

Buffer Overread in selftest_pbkdf2() in kdf.c/libgcrypt
Closed, InvalidPublic
Actions

Description

There is a buffer overread in the selftest_pbkdf function in libgcrypt.

The bug is triggered in this line:

for (tvidx=0; tv[tvidx].desc; tvidx++)

From what I can tell the idea here is to iterate over the tv array until it hits an element where desc is set to NULL. I.e. it assumes there is a terminating element with everything set to zero. However there is no such thing, so it overreads.

I'm attaching a patch.

libgcrypt-fix-overflow-test-suite.diff488 BDownload

I have to say I find this very concerning. The bug itself is not very relevant. However this is a bug triggered by the testsuite and can easily be detected by running tests with asan. Such a bug should be automatically discovered before it hits a release. Testing code with asan is imho one of the most basic security hygiene measurements for C-based software that I can think of.

I don't see any indication that libgcrypt is using any kind of CI. It probably should have one that is running the test suite with various safety and bug detection features enabled.

Related Objects

Event Timeline

hanno created this task.Jan 29 2021, 1:35 PM
bevan added a subscriber: bevan.Jan 29 2021, 1:52 PM

This should already be fixed in libgcrypt 1.9.1 by rCc6425a5537294dfe2beaafc9105f7af4ceac677f.

hanno added a comment.Jan 29 2021, 2:15 PM

Yeah looks like a duplicate. You may rename the bug to "Please implement some basic safety checks in a CI".

• werner closed this task as Invalid.Jan 29 2021, 2:38 PM
• werner added a project: Duplicate.
• werner added a subscriber: • werner.

@hanno, this is a bug tracker and not yet another media for your rants.

I would also have expected that you are able to follow the usual bug reporting rules (see for example Libgcrypt 's NEWS ) before posting a report again.

ilf added a subscriber: ilf.Jan 30 2021, 5:42 PM
aheinecke added a subscriber: aheinecke.Feb 1 2021, 6:32 PM

Hi,

the issue regarding this self test was immediately found after release. Our development is completely open and everyone is free to run tests with our software on any platform at any time. We would respect and fix all those bug reports. None about this reached us during the development phase.
As this is not happening as it should during development we release and test on our platforms and build systems. When after the release others test, too we immediately fix the issues as happened with 1.9.1 in libgcrypt.

We are working on better test coverage apart from our main developer systems but this is hard esp. with MacOS and their mutlitple development platforms and we are also not just saying we support Linux but we support a multitude of UNIXes. This is not trivial.

aheinecke added a comment.Feb 1 2021, 6:34 PM

For what it is worth we have also just tasked someone from our team to reinstate our buildbot / CI but this would likely not have helped in the current case of the libgcrypt buffer error as only ASAN with large hashtests would have found this. Still we have the general infrastructure for such tests we are just lacking resources. That is why we publish everything and encourage the community to at least help us with testing.

bernhard added a subscriber: bernhard.Feb 1 2021, 6:46 PM

Hello,

to explain a bit more: This report was opened after the reported defect was already fixed.
As we are getting many reports and technical suggestions, please keep the reports focused on one point only if possible
and open general discussion points about development improvements on gnupg-devel@.

The suggestion to refocus the issue to something that is perceived as a broad criticism is not helpful, as
it is creating more work and distracts from the initially reported issue.
(As usual with Free Software communities, we from GnuPG try to guide the incoming
reports, so they can be handled in the best way, which profits everybody.)

Again, thanks for reporting and taking the time to examine GnuPG!
We appreciate feedback and suggestions (but please help us to manage them, so we can respond in helpful ways).

Best Regards,
Bernhard