diff options
author | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2021-03-30 22:03:49 +0200 |
---|---|---|
committer | Heiko Schlittermann (HS12-RIPE) <hs@schlittermann.de> | 2021-05-27 21:30:50 +0200 |
commit | c82e60b402bd17620e57a0774d27b39d7ea6eb09 (patch) | |
tree | 37aa66f23b18c940c462e7fbdc0c70cac0781e0d /src | |
parent | d17c916db7c661aacf65684a5568f8e105e50b3b (diff) |
CVE-2020-28026: Line truncation and injection in spool_read_header()
This also fixes:
2/ In src/spool_in.c:
462 while ( (len = Ustrlen(big_buffer)) == big_buffer_size-1
463 && big_buffer[len-1] != '\n'
464 )
465 { /* buffer not big enough for line; certs make this possible */
466 uschar * buf;
467 if (big_buffer_size >= BIG_BUFFER_SIZE*4) goto SPOOL_READ_ERROR;
468 buf = store_get_perm(big_buffer_size *= 2, FALSE);
469 memcpy(buf, big_buffer, --len);
The --len in memcpy() chops off a useful byte (we know for sure that
big_buffer[len-1] is not a '\n' because we entered the while loop).
Based on a patch done by Qualys.
(cherry picked from commit f0c307458e1ee81abbe7ed2d4a8d16b5cbd8a799)
(cherry picked from commit 4daba4bec729a57fb0863af786a1395e70794c76)
Diffstat (limited to 'src')
-rw-r--r-- | src/src/spool_in.c | 48 |
1 files changed, 32 insertions, 16 deletions
diff --git a/src/src/spool_in.c b/src/src/spool_in.c index 09fe9c5f7..0bebcea97 100644 --- a/src/src/spool_in.c +++ b/src/src/spool_in.c @@ -304,6 +304,35 @@ dsn_ret = 0; dsn_envid = NULL; } +static void * +fgets_big_buffer(FILE *fp) +{ +int len = 0; + +big_buffer[0] = 0; +if (Ufgets(big_buffer, big_buffer_size, fp) == NULL) return NULL; + +while ((len = Ustrlen(big_buffer)) == big_buffer_size-1 + && big_buffer[len-1] != '\n') + { + uschar *newbuffer; + int newsize; + + if (big_buffer_size >= BIG_BUFFER_SIZE * 4) return NULL; + newsize = big_buffer_size * 2; + newbuffer = store_get_perm(newsize, FALSE); + memcpy(newbuffer, big_buffer, len); + + big_buffer = newbuffer; + big_buffer_size = newsize; + if (Ufgets(big_buffer + len, big_buffer_size - len, fp) == NULL) return NULL; + } + +if (len <= 0 || big_buffer[len-1] != '\n') return NULL; +return big_buffer; +} + + /************************************************* * Read spool header file * @@ -454,26 +483,13 @@ If the line starts with "--" the content of the variable is tainted. */ for (;;) { - int len; BOOL tainted; uschar * var; const uschar * p; - if (Ufgets(big_buffer, big_buffer_size, fp) == NULL) goto SPOOL_READ_ERROR; + if (fgets_big_buffer(fp) == NULL) goto SPOOL_READ_ERROR; if (big_buffer[0] != '-') break; - while ( (len = Ustrlen(big_buffer)) == big_buffer_size-1 - && big_buffer[len-1] != '\n' - ) - { /* buffer not big enough for line; certs make this possible */ - uschar * buf; - if (big_buffer_size >= BIG_BUFFER_SIZE*4) goto SPOOL_READ_ERROR; - buf = store_get_perm(big_buffer_size *= 2, FALSE); - memcpy(buf, big_buffer, len); - big_buffer = buf; - if (Ufgets(big_buffer+len, big_buffer_size-len, fp) == NULL) - goto SPOOL_READ_ERROR; - } - big_buffer[len-1] = 0; + big_buffer[Ustrlen(big_buffer)-1] = 0; tainted = big_buffer[1] == '-'; var = big_buffer + (tainted ? 2 : 1); @@ -774,7 +790,7 @@ for (recipients_count = 0; recipients_count < rcount; recipients_count++) uschar *errors_to = NULL; uschar *p; - if (Ufgets(big_buffer, big_buffer_size, fp) == NULL) goto SPOOL_READ_ERROR; + if (fgets_big_buffer(fp) == NULL) goto SPOOL_READ_ERROR; nn = Ustrlen(big_buffer); if (nn < 2) goto SPOOL_FORMAT_ERROR; |