clamav: fix CVE-2024-20505 & CVE-2024-20506

Backport fixes for:

* CVE-2024-20505 - Upstream-Status: Backport from https://github.com/Cisco-Talos/clamav/commit/8915bd22570ee608907f1b88a68e587d17813812
* CVE-2024-20506 - Upstream-Status: Backport from https://github.com/Cisco-Talos/clamav/commit/88efeda2a4cb93a69cf0994c02a8987f06fa204d

Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>
This commit is contained in:
Hitendra Prajapati
2024-09-06 13:49:40 +05:30
committed by Armin Kuster
parent 2d5aaeb705
commit e0bfd27ca0
3 changed files with 244 additions and 0 deletions
@@ -20,6 +20,8 @@ SRC_URI = "git://github.com/Cisco-Talos/clamav;branch=rel/0.104;protocol=https \
file://tmpfiles.clamav \ file://tmpfiles.clamav \
file://headers_fixup.patch \ file://headers_fixup.patch \
file://oe_cmake_fixup.patch \ file://oe_cmake_fixup.patch \
file://CVE-2024-20505.patch \
file://CVE-2024-20506.patch \
" "
S = "${WORKDIR}/git" S = "${WORKDIR}/git"
@@ -0,0 +1,118 @@
From 8915bd22570ee608907f1b88a68e587d17813812 Mon Sep 17 00:00:00 2001
From: Micah Snyder <micasnyd@cisco.com>
Date: Tue, 16 Jul 2024 11:22:05 -0400
Subject: [PATCH] Fix possible out of bounds read in PDF parser
The `find_length()` function in the PDF parser incorrectly assumes that
objects found are located in the main PDF file map, and fails to take
into account whether the objects were in fact found in extracted PDF
object streams. The resulting pointer is then invalid and may be an out
of bounds read.
This issue was found by OSS-Fuzz.
This fix checks if the object is from an object stream, and then
calculates the pointer based on the start of the object stream instead
of based on the start of the PDF.
I've also added extra checks to verify the calculated pointer and object
size are within the stream (or PDF file map). I'm not entirely sure this
is necessary, but better safe than sorry.
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=69617
Upstream-Status: Backport [https://github.com/Cisco-Talos/clamav/commit/8915bd22570ee608907f1b88a68e587d17813812]
CVE: CVE-2024-20505
Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
---
libclamav/pdf.c | 45 +++++++++++++++++++++++++++++++++++++++------
libclamav/pdfng.c | 5 +++++
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/libclamav/pdf.c b/libclamav/pdf.c
index 01f32e07a..40eea19eb 100644
--- a/libclamav/pdf.c
+++ b/libclamav/pdf.c
@@ -1009,8 +1009,26 @@ static size_t find_length(struct pdf_struct *pdf, struct pdf_obj *obj, const cha
return 0;
}
- indirect_obj_start = pdf->map + obj->start;
- bytes_remaining = pdf->size - obj->start;
+ if (NULL == obj->objstm) {
+ indirect_obj_start = (const char *)(obj->start + pdf->map);
+
+ if (!CLI_ISCONTAINED(pdf->map, pdf->size, indirect_obj_start, obj->size)) {
+ cli_dbgmsg("find_length: indirect object found, but not contained in PDF\n");
+ return 0;
+ }
+
+ bytes_remaining = pdf->size - obj->start;
+
+ } else {
+ indirect_obj_start = (const char *)(obj->start + obj->objstm->streambuf);
+
+ if (!CLI_ISCONTAINED(obj->objstm->streambuf, obj->objstm->streambuf_len, indirect_obj_start, obj->size)) {
+ cli_dbgmsg("find_length: indirect object found, but not contained in PDF streambuf\n");
+ return 0;
+ }
+
+ bytes_remaining = obj->objstm->streambuf_len - obj->start;
+ }
/* Ok so we found the indirect object, lets read the value. */
index = pdf_nextobject(indirect_obj_start, bytes_remaining);
@@ -3093,15 +3111,30 @@ void pdf_handle_enc(struct pdf_struct *pdf)
obj = find_obj(pdf, pdf->objs[0], pdf->enc_objid);
if (!obj) {
- cli_dbgmsg("pdf_handle_enc: can't find encrypted object %d %d\n", pdf->enc_objid >> 8, pdf->enc_objid & 0xff);
- noisy_warnmsg("pdf_handle_enc: can't find encrypted object %d %d\n", pdf->enc_objid >> 8, pdf->enc_objid & 0xff);
+ cli_dbgmsg("pdf_handle_enc: can't find encryption object %d %d\n", pdf->enc_objid >> 8, pdf->enc_objid & 0xff);
+ noisy_warnmsg("pdf_handle_enc: can't find encryption object %d %d\n", pdf->enc_objid >> 8, pdf->enc_objid & 0xff);
return;
}
len = obj->size;
- q = (obj->objstm) ? (const char *)(obj->start + obj->objstm->streambuf)
- : (const char *)(obj->start + pdf->map);
+ if (NULL == obj->objstm) {
+ q = (const char *)(obj->start + pdf->map);
+
+ if (!CLI_ISCONTAINED(pdf->map, pdf->size, q, len)) {
+ cli_dbgmsg("pdf_handle_enc: encryption object found, but not contained in PDF\n");
+ noisy_warnmsg("pdf_handle_enc: encryption object found, but not contained in PDF\n");
+ return;
+ }
+ } else {
+ q = (const char *)(obj->start + obj->objstm->streambuf);
+
+ if (!CLI_ISCONTAINED(obj->objstm->streambuf, obj->objstm->streambuf_len, q, len)) {
+ cli_dbgmsg("pdf_handle_enc: encryption object found, but not contained in PDF streambuf\n");
+ noisy_warnmsg("pdf_handle_enc: encryption object found, but not contained in PDF streambuf\n");
+ return;
+ }
+ }
O = U = UE = StmF = StrF = EFF = NULL;
do {
diff --git a/libclamav/pdfng.c b/libclamav/pdfng.c
index a5daa2891..977a95e4d 100644
--- a/libclamav/pdfng.c
+++ b/libclamav/pdfng.c
@@ -450,6 +450,11 @@ char *pdf_parse_string(struct pdf_struct *pdf, struct pdf_obj *obj, const char *
if (!(newobj))
return NULL;
+ if (!CLI_ISCONTAINED(pdf->map, pdf->size, newobj->start, newobj->size)) {
+ cli_dbgmsg("pdf_parse_string: object not contained in PDF\n");
+ return NULL;
+ }
+
if (newobj == obj)
return NULL;
--
2.25.1
@@ -0,0 +1,124 @@
From 88efeda2a4cb93a69cf0994c02a8987f06fa204d Mon Sep 17 00:00:00 2001
From: Micah Snyder <micasnyd@cisco.com>
Date: Mon, 26 Aug 2024 14:00:51 -0400
Subject: [PATCH] Disable following symlinks when opening log files
The log module used by clamd and freshclam may follow symlinks.
This is a potential security concern since the log may be owned by
the unprivileged service but may be opened by the service running as
root on startup.
For Windows, we'll define O_NOFOLLOW so the code works, though the issue
does not affect Windows.
Issue reported by Detlef.
Upstream-Status: Backport [https://github.com/Cisco-Talos/clamav/commit/88efeda2a4cb93a69cf0994c02a8987f06fa204d]
CVE: CVE-2024-20506
Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
---
common/output.c | 51 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 40 insertions(+), 11 deletions(-)
diff --git a/common/output.c b/common/output.c
index 968cea09f..f3ea7f980 100644
--- a/common/output.c
+++ b/common/output.c
@@ -58,6 +58,12 @@
#include "output.h"
+// Define O_NOFOLLOW for systems that don't have it.
+// Notably, Windows doesn't have O_NOFOLLOW.
+#ifndef O_NOFOLLOW
+#define O_NOFOLLOW 0
+#endif
+
#ifdef CL_THREAD_SAFE
#include <pthread.h>
pthread_mutex_t logg_mutex = PTHREAD_MUTEX_INITIALIZER;
@@ -323,7 +329,6 @@ int logg(const char *str, ...)
char buffer[1025], *abuffer = NULL, *buff;
time_t currtime;
size_t len;
- mode_t old_umask;
#ifdef F_WRLCK
struct flock fl;
#endif
@@ -357,18 +362,36 @@ int logg(const char *str, ...)
logg_open();
if (!logg_fp && logg_file) {
- old_umask = umask(0037);
- if ((logg_fp = fopen(logg_file, "at")) == NULL) {
- umask(old_umask);
+ int logg_file_fd = -1;
+
+ logg_file_fd = open(logg_file, O_WRONLY | O_CREAT | O_APPEND | O_NOFOLLOW, 0640);
+ if (-1 == logg_file_fd) {
+ char errbuf[128];
+ cli_strerror(errno, errbuf, sizeof(errbuf));
+ printf("ERROR: Failed to open log file %s: %s\n", logg_file, errbuf);
+
#ifdef CL_THREAD_SAFE
pthread_mutex_unlock(&logg_mutex);
#endif
- printf("ERROR: Can't open %s in append mode (check permissions!).\n", logg_file);
- if (len > sizeof(buffer))
+ if (abuffer)
free(abuffer);
return -1;
- } else
- umask(old_umask);
+ }
+
+ logg_fp = fdopen(logg_file_fd, "at");
+ if (NULL == logg_fp) {
+ char errbuf[128];
+ cli_strerror(errno, errbuf, sizeof(errbuf));
+ printf("ERROR: Failed to convert the open log file descriptor for %s to a FILE* handle: %s\n", logg_file, errbuf);
+
+ close(logg_file_fd);
+#ifdef CL_THREAD_SAFE
+ pthread_mutex_unlock(&logg_mutex);
+#endif
+ if (abuffer)
+ free(abuffer);
+ return -1;
+ }
#ifdef F_WRLCK
if (logg_lock) {
@@ -381,11 +404,16 @@ int logg(const char *str, ...)
else
#endif
{
+ char errbuf[128];
+ cli_strerror(errno, errbuf, sizeof(errbuf));
+ printf("ERROR: Failed to lock the log file %s: %s\n", logg_file, errbuf);
+
#ifdef CL_THREAD_SAFE
pthread_mutex_unlock(&logg_mutex);
#endif
- printf("ERROR: %s is locked by another process\n", logg_file);
- if (len > sizeof(buffer))
+ fclose(logg_fp);
+ logg_fp = NULL;
+ if (abuffer)
free(abuffer);
return -1;
}
@@ -462,8 +490,9 @@ int logg(const char *str, ...)
pthread_mutex_unlock(&logg_mutex);
#endif
- if (len > sizeof(buffer))
+ if (abuffer)
free(abuffer);
+
return 0;
}
--
2.25.1