Page MenuHome GnuPG
Create Task
Maniphest T6455

Bug in regexp library may lead to out-of-bounds read
Closed, ResolvedPublic
Actions

Description

The compiler in the bundled regular expression library allows any character to end a POSIX character class expression. Besides incorrect parsing, this can involve skipping the string terminator in the face of a malformed expression.
Test case:

#include "jimregexp.h"

int
main (void)
{
  regex_t r;
  return !regcomp (&r, "[[:digit:\0]", 0);
}

Trivial fix:

diff --git a/regexp/jimregexp.c b/regexp/jimregexp.c
index 7fd6d473e..1a8b8aae6 100644
--- a/regexp/jimregexp.c
+++ b/regexp/jimregexp.c
@@ -795,7 +795,8 @@ static int regatom(regex_t *preg, int *flagp)
 
                                        for (cc = 0; cc < CC_NUM; cc++) {
                                                n = strlen(character_class[cc]);
-                                               if (strncmp(pattern, character_class[cc], n) == 0) {
+                                               if (strncmp(pattern, character_class[cc], n) == 0
+                                                    && pattern[n] == ']') {
                                                        /* Found a character class */
                                                        pattern += n + 1;
                                                        break;

I have not investigated whether this has any impact on current versions of GnuPG.

Revisions and Commits

rG GnuPG
rG762b7d07eaa8 common: Incorporate upstream changes of regexp.
rG464e85d43596 common: Incorporate upstream changes of regexp.
rG3ad4b339b886 common: Fix minor bug in the jimregexp code.
rGa82e6f310a03 common: Fix minor bug in the jimregexp code.

Event Timeline

Guldrelokk created this task.Apr 18 2023, 12:20 PM
Guldrelokk added a comment.Apr 18 2023, 12:24 PM

Another miscellaneous correction for jimregexp. A condition was copy-pasted from another section without the necessary changes, resulting in incorrect logic. This seems harmless apart from inconsistent error reporting.

diff --git a/regexp/jimregexp.c b/regexp/jimregexp.c
index 1a8b8aae6..1b6e1b49c 100644
--- a/regexp/jimregexp.c
+++ b/regexp/jimregexp.c
@@ -778,7 +778,7 @@ static int regatom(regex_t *preg, int *flagp)
                                                        preg->err = REG_ERR_NULL_CHAR;
                                                        return 0;
                                                }
-                                               if (start == '\\' && *pattern == 0) {
+                                               if (end == '\\' && *pattern == 0) {
                                                        preg->err = REG_ERR_INVALID_ESCAPE;
                                                        return 0;
                                                }
• werner assigned this task to • gniibe.Apr 18 2023, 5:11 PM
• werner triaged this task as High priority.
• werner edited projects, added gnupg22, gnupg24; removed gnupg.
• werner added subscribers: • gniibe, • werner.

@gniibe, will you be so kind an check the provided patches

• werner claimed this task.Apr 20 2023, 12:17 PM
• werner added a commit: rGa82e6f310a03: common: Fix minor bug in the jimregexp code..Apr 20 2023, 12:28 PM
• werner added a commit: rG3ad4b339b886: common: Fix minor bug in the jimregexp code..
• werner closed this task as Resolved.Apr 20 2023, 12:29 PM
• werner moved this task from Backlog to QA on the gnupg22 board.

Okay, that was easy to check.

• werner moved this task from Backlog to QA on the gnupg24 board.Apr 20 2023, 12:29 PM
• werner moved this task from QA to gnupg-2.4.1 on the gnupg24 board.
• werner edited projects, added gnupg24 (gnupg-2.4.1); removed gnupg24.
• werner moved this task from QA to gnupg-2.2.42 on the gnupg22 board.
• werner edited projects, added gnupg22 (gnupg-2.2.42); removed gnupg22.
• gniibe added a comment.Apr 21 2023, 5:03 AM

I checked the upstream. For the reported issue, upstream version raises an error with REG_ERR_UNMATCHED_BRACKET.
That behavior is better (as we don't have particular reason to maintain different behavior from upstream version).
Also, I found another change from upstream for end of word check.

I'm going to incorporate those change.

• gniibe added a commit: rG464e85d43596: common: Incorporate upstream changes of regexp..Apr 21 2023, 5:04 AM
• gniibe added a commit: rG762b7d07eaa8: common: Incorporate upstream changes of regexp..