Skip to content

Commit 6eea7a2

Browse files
committed
Libclamav: copy OOXML content-type attributes
OOXML metadata parsing reads [Content_Types].xml with libxml2. The parser kept pointers returned by xmlTextReaderConstValue() for ContentType and PartName after continuing through the attribute list. libxml2 may reuse or reallocate that reader-owned storage on later reader calls, so those pointers can become stale before they are used for ZIP entry lookup. Copy the attribute values that need to outlive the current reader call and free them when moving to the next Override or leaving the parser. Treat allocation failure as CL_EMEM. We do not believe this to be a security concern based on current evidence. The stale value is only read as a string length/name for metadata-mode OOXML processing, metadata collection is off by default, and a standard non-ASan build did not crash with the supplied sample. Reported by David Pokora and Evan Sultanik of Trail of Bits, working with Anthropic. CLAM-2994
1 parent d7ab363 commit 6eea7a2

1 file changed

Lines changed: 45 additions & 15 deletions

File tree

libclamav/ooxml.c

Lines changed: 45 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -176,13 +176,31 @@ static cl_error_t ooxml_extn_cb(int fd, const char *filepath, cli_ctx *ctx, cons
176176
return ret;
177177
}
178178

179+
/**
180+
* Copy an XML attribute value that must outlive the current text reader call.
181+
*/
182+
static cl_error_t ooxml_copy_attr_value(xmlChar **dst, const xmlChar *value)
183+
{
184+
xmlChar *copy = xmlStrdup(value);
185+
186+
if (copy == NULL)
187+
return CL_EMEM;
188+
189+
xmlFree(*dst);
190+
*dst = copy;
191+
192+
return CL_SUCCESS;
193+
}
194+
179195
static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, const char *name, uint32_t attributes)
180196
{
181-
cl_error_t ret = CL_SUCCESS;
197+
cl_error_t status = CL_SUCCESS;
198+
cl_error_t ret = CL_SUCCESS;
182199
int tmp, toval = 0, state;
183200
int core = 0, extn = 0, cust = 0, dsig = 0;
184201
int mcore = 0, mextn = 0, mcust = 0;
185-
const xmlChar *localname, *value, *CT, *PN;
202+
const xmlChar *localname, *value;
203+
xmlChar *CT = NULL, *PN = NULL;
186204
xmlTextReaderPtr reader = NULL;
187205
uint32_t loff;
188206

@@ -216,8 +234,8 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
216234
/* locate core-properties, extended-properties, and custom-properties (optional) */
217235
while ((state = xmlTextReaderRead(reader)) == 1) {
218236
if (cli_json_timeout_cycle_check(ctx, &toval) != CL_SUCCESS) {
219-
ret = CL_ETIMEOUT;
220-
goto ooxml_content_exit;
237+
status = CL_ETIMEOUT;
238+
goto done;
221239
}
222240

223241
localname = xmlTextReaderConstLocalName(reader);
@@ -227,16 +245,26 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
227245

228246
if (xmlTextReaderHasAttributes(reader) != 1) continue;
229247

248+
xmlFree(CT);
249+
xmlFree(PN);
230250
CT = PN = NULL;
231251
while (xmlTextReaderMoveToNextAttribute(reader) == 1) {
232252
localname = xmlTextReaderConstLocalName(reader);
233253
value = xmlTextReaderConstValue(reader);
234254
if (localname == NULL || value == NULL) continue;
235255

236256
if (!xmlStrcmp(localname, (const xmlChar *)"ContentType")) {
237-
CT = value;
257+
ret = ooxml_copy_attr_value(&CT, value);
258+
if (ret != CL_SUCCESS) {
259+
status = ret;
260+
goto done;
261+
}
238262
} else if (!xmlStrcmp(localname, (const xmlChar *)"PartName")) {
239-
PN = value;
263+
ret = ooxml_copy_attr_value(&PN, value);
264+
if (ret != CL_SUCCESS) {
265+
status = ret;
266+
goto done;
267+
}
240268
}
241269

242270
cli_dbgmsg("%s: %s\n", localname, value);
@@ -248,7 +276,7 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
248276
/* default: /docProps/core.xml*/
249277
tmp = unzip_search_single(ctx, (const char *)(PN + 1), xmlStrlen(PN) - 1, &loff);
250278
if (tmp == CL_ETIMEOUT) {
251-
ret = tmp;
279+
status = tmp;
252280
} else if (tmp != CL_VIRUS) {
253281
cli_dbgmsg("cli_process_ooxml: failed to find core properties file \"%s\"!\n", PN);
254282
mcore++;
@@ -257,7 +285,7 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
257285
if (!core) {
258286
tmp = unzip_single_internal(ctx, loff, ooxml_core_cb);
259287
if (tmp == CL_ETIMEOUT || tmp == CL_EMEM) {
260-
ret = tmp;
288+
status = tmp;
261289
}
262290
}
263291
core++;
@@ -266,7 +294,7 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
266294
/* default: /docProps/app.xml */
267295
tmp = unzip_search_single(ctx, (const char *)(PN + 1), xmlStrlen(PN) - 1, &loff);
268296
if (tmp == CL_ETIMEOUT) {
269-
ret = tmp;
297+
status = tmp;
270298
} else if (tmp != CL_VIRUS) {
271299
cli_dbgmsg("cli_process_ooxml: failed to find extended properties file \"%s\"!\n", PN);
272300
mextn++;
@@ -275,7 +303,7 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
275303
if (!extn) {
276304
tmp = unzip_single_internal(ctx, loff, ooxml_extn_cb);
277305
if (tmp == CL_ETIMEOUT || tmp == CL_EMEM) {
278-
ret = tmp;
306+
status = tmp;
279307
}
280308
}
281309
extn++;
@@ -284,7 +312,7 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
284312
/* default: /docProps/custom.xml */
285313
tmp = unzip_search_single(ctx, (const char *)(PN + 1), xmlStrlen(PN) - 1, &loff);
286314
if (tmp == CL_ETIMEOUT) {
287-
ret = tmp;
315+
status = tmp;
288316
} else if (tmp != CL_VIRUS) {
289317
cli_dbgmsg("cli_process_ooxml: failed to find custom properties file \"%s\"!\n", PN);
290318
mcust++;
@@ -297,11 +325,11 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
297325
dsig++;
298326
}
299327

300-
if (ret != CL_SUCCESS)
301-
goto ooxml_content_exit;
328+
if (status != CL_SUCCESS)
329+
goto done;
302330
}
303331

304-
ooxml_content_exit:
332+
done:
305333
if (core) {
306334
cli_jsonint(ctx->this_layer_metadata_json, "CorePropertiesFileCount", core);
307335
if (core > 1)
@@ -343,9 +371,11 @@ static cl_error_t ooxml_content_cb(int fd, const char *filepath, cli_ctx *ctx, c
343371
ctx->scansize = sav_scansize;
344372
ctx->scannedfiles = sav_scannedfiles;
345373

374+
xmlFree(CT);
375+
xmlFree(PN);
346376
xmlTextReaderClose(reader);
347377
xmlFreeTextReader(reader);
348-
return ret;
378+
return status;
349379
}
350380

351381
static cl_error_t ooxml_hwp_cb(int fd, const char *filepath, cli_ctx *ctx, const char *name, uint32_t attributes)

0 commit comments

Comments
 (0)