build_SYS_str_reasons: Fix a crash caused by overlong locales
authorVitezslav Cizek <vcizek@suse.com>
Tue, 4 Jun 2019 11:24:59 +0000 (13:24 +0200)
committerTomas Mraz <tmraz@fedoraproject.org>
Tue, 4 Jun 2019 11:30:36 +0000 (13:30 +0200)
The 4 kB SPACE_SYS_STR_REASONS in crypto/err/err.c isn't enough for some locales.
The Russian locales consume 6856 bytes, Ukrainian even 7000.

build_SYS_str_reasons() contains an overflow check:

if (cnt > sizeof(strerror_pool))
    cnt = sizeof(strerror_pool);

But since commit 9f15e5b911ba6053e09578f190354568e01c07d7 it no longer
works as cnt is incremented once more after the condition.

cnt greater than sizeof(strerror_pool) results in an unbounded
OPENSSL_strlcpy() in openssl_strerror_r(), eventually causing a crash.

When the first received error string was empty or contained only
spaces, cur would move in front of the start of the strerror_pool.

Also don't call openssl_strerror_r when the pool is full.

Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/8966)

(cherry picked from commit fac9200a881a83bef038ebed628ebd409786a1a6)

crypto/err/err.c

index eaf6712fdbd02bcb1bc9a15deb7f3542e6a25c42..3a58ccb9588296ccea3814824d64f62ccd727fa0 100644 (file)
@@ -184,8 +184,8 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)
 }
 
 #ifndef OPENSSL_NO_ERR
-/* A measurement on Linux 2018-11-21 showed about 3.5kib */
-# define SPACE_SYS_STR_REASONS 4 * 1024
+/* 2019-05-21: Russian and Ukrainian locales on Linux require more than 6,5 kB */
+# define SPACE_SYS_STR_REASONS 8 * 1024
 # define NUM_SYS_STR_REASONS 127
 
 static ERR_STRING_DATA SYS_str_reasons[NUM_SYS_STR_REASONS + 1];
@@ -219,21 +219,23 @@ static void build_SYS_str_reasons(void)
         ERR_STRING_DATA *str = &SYS_str_reasons[i - 1];
 
         str->error = ERR_PACK(ERR_LIB_SYS, 0, i);
-        if (str->string == NULL) {
+        /*
+         * If we have used up all the space in strerror_pool,
+         * there's no point in calling openssl_strerror_r()
+         */
+        if (str->string == NULL && cnt < sizeof(strerror_pool)) {
             if (openssl_strerror_r(i, cur, sizeof(strerror_pool) - cnt)) {
                 size_t l = strlen(cur);
 
                 str->string = cur;
                 cnt += l;
-                if (cnt > sizeof(strerror_pool))
-                    cnt = sizeof(strerror_pool);
                 cur += l;
 
                 /*
                  * VMS has an unusual quirk of adding spaces at the end of
-                 * some (most? all?) messages.  Lets trim them off.
+                 * some (most? all?) messages. Lets trim them off.
                  */
-                while (ossl_isspace(cur[-1])) {
+                while (cur > strerror_pool && ossl_isspace(cur[-1])) {
                     cur--;
                     cnt--;
                 }