From: Ken Sharp Date: Thu, 25 Jan 2024 11:53:44 +0000 Subject: Bug 707510 - review printing of pointers Origin: https://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=ff1013a0ab485b66783b70145e342a82c670906a Bug: https://bugs.ghostscript.com/show_bug.cgi?id=707510 Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2024-29508 This is for item 4 of the report, which is addressed by the change in gdevpdtb.c. That change uses a fixed name for fonts which have no name instead of using the pointer to the address of the font. The remaining changes are all due to reviewing the use of PRI_INTPTR. In general we only use that for debugging purposes but there were a few places which were printing pointers arbitrarily, even in a release build. We really don't want to do that so I've modified the places which were printing pointer unconditionally so that they only do so if DEBUG is set at compile time, or a specific debug flag is set. --- base/gsfont.c | 4 ++-- base/gsicc_cache.c | 8 ++++---- base/gsmalloc.c | 4 ++-- base/gxclmem.c | 5 ++--- base/gxcpath.c | 6 +++++- base/gxpath.c | 8 +++++++- base/szlibc.c | 4 +++- devices/gdevupd.c | 7 ++++++- devices/vector/gdevpdtb.c | 4 ++-- psi/ialloc.c | 4 ++-- psi/igc.c | 6 +++--- psi/igcstr.c | 6 +++--- psi/iinit.c | 6 +++++- psi/imainarg.c | 5 +++-- psi/isave.c | 4 ++-- psi/iutil.c | 6 +++++- 16 files changed, 56 insertions(+), 31 deletions(-) --- a/base/gsfont.c +++ b/base/gsfont.c @@ -791,7 +791,7 @@ gs_purge_font(gs_font * pfont) else if (pdir->scaled_fonts == pfont) pdir->scaled_fonts = next; else { /* Shouldn't happen! */ - lprintf1("purged font "PRI_INTPTR" not found\n", (intptr_t)pfont); + if_debug1m('u', pfont->memory, "purged font "PRI_INTPTR" not found\n", (intptr_t)pfont); } /* Purge the font from the scaled font cache. */ --- a/base/gsicc_cache.c +++ b/base/gsicc_cache.c @@ -161,7 +161,7 @@ icc_linkcache_finalize(const gs_memory_t return; while (link_cache->head != NULL) { if (link_cache->head->ref_count != 0) { - emprintf2(link_cache->memory, "link at "PRI_INTPTR" being removed, but has ref_count = %d\n", + if_debug2m(gs_debug_flag_icc, link_cache->memory, "link at "PRI_INTPTR" being removed, but has ref_count = %d\n", (intptr_t)link_cache->head, link_cache->head->ref_count); link_cache->head->ref_count = 0; /* force removal */ } @@ -586,7 +586,7 @@ gsicc_findcachelink(gsicc_hashlink_t has /* that was building it failed to be able to complete building it. Try this only a limited number of times before we bail. */ if (curr->valid == false) { - emprintf1(curr->memory, "link "PRI_INTPTR" lock released, but still not valid.\n", (intptr_t)curr); /* Breakpoint here */ + if_debug1m(gs_debug_flag_icc, curr->memory, "link "PRI_INTPTR" lock released, but still not valid.\n", (intptr_t)curr); /* Breakpoint here */ } gx_monitor_enter(icc_link_cache->lock); /* re-enter to loop and check */ } @@ -614,7 +614,7 @@ gsicc_remove_link(gsicc_link_t *link) /* NOTE: link->ref_count must be 0: assert ? */ gx_monitor_enter(icc_link_cache->lock); if (link->ref_count != 0) { - emprintf2(memory, "link at "PRI_INTPTR" being removed, but has ref_count = %d\n", (intptr_t)link, link->ref_count); + if_debug2m(gs_debug_flag_icc, memory, "link at "PRI_INTPTR" being removed, but has ref_count = %d\n", (intptr_t)link, link->ref_count); } curr = icc_link_cache->head; prev = NULL; --- a/base/gsmalloc.c +++ b/base/gsmalloc.c @@ -420,7 +420,7 @@ gs_heap_resize_string(gs_memory_t * mem, client_name_t cname) { if (gs_heap_object_type(mem, data) != &st_bytes) - lprintf2("%s: resizing non-string "PRI_INTPTR"!\n", + if_debug2m('a', mem, "%s: resizing non-string "PRI_INTPTR"!\n", client_name_string(cname), (intptr_t)data); return gs_heap_resize_object(mem, data, new_num, cname); } --- a/base/gxclmem.c +++ b/base/gxclmem.c @@ -490,8 +490,7 @@ memfile_fclose(clist_file_ptr cf, const /* leaks if other users of the memfile don't 'fclose with delete=true */ if (f->openlist != NULL || ((f->base_memfile != NULL) && f->base_memfile->is_open)) { /* TODO: do the cleanup rather than just giving an error */ - emprintf1(f->memory, - "Attempt to delete a memfile still open for read: "PRI_INTPTR"\n", + if_debug1(':', "Attempt to delete a memfile still open for read: "PRI_INTPTR"\n", (intptr_t)f); return_error(gs_error_invalidfileaccess); } else { --- a/base/gxcpath.c +++ b/base/gxcpath.c @@ -175,8 +175,10 @@ gx_cpath_init_contained_shared(gx_clip_p { if (shared) { if (shared->path.segments == &shared->path.local_segments) { +#ifdef DEBUG lprintf1("Attempt to share (local) segments of clip path "PRI_INTPTR"!\n", (intptr_t)shared); +#endif return_error(gs_error_Fatal); } *pcpath = *shared; @@ -233,8 +235,10 @@ gx_cpath_init_local_shared_nested(gx_cli if (shared) { if ((shared->path.segments == &shared->path.local_segments) && !safely_nested) { +#ifdef DEBUG lprintf1("Attempt to share (local) segments of clip path "PRI_INTPTR"!\n", (intptr_t)shared); +#endif return_error(gs_error_Fatal); } pcpath->path = shared->path; --- a/base/gxpath.c +++ b/base/gxpath.c @@ -137,8 +137,10 @@ gx_path_init_contained_shared(gx_path * { if (shared) { if (shared->segments == &shared->local_segments) { +#ifdef DEBUG lprintf1("Attempt to share (local) segments of path "PRI_INTPTR"!\n", (intptr_t)shared); +#endif return_error(gs_error_Fatal); } *ppath = *shared; @@ -172,8 +174,10 @@ gx_path_alloc_shared(const gx_path * sha ppath->procs = &default_path_procs; if (shared) { if (shared->segments == &shared->local_segments) { +#ifdef DEBUG lprintf1("Attempt to share (local) segments of path "PRI_INTPTR"!\n", (intptr_t)shared); +#endif gs_free_object(mem, ppath, cname); return 0; } @@ -203,8 +207,10 @@ gx_path_init_local_shared(gx_path * ppat { if (shared) { if (shared->segments == &shared->local_segments) { +#ifdef DEBUG lprintf1("Attempt to share (local) segments of path "PRI_INTPTR"!\n", (intptr_t)shared); +#endif return_error(gs_error_Fatal); } *ppath = *shared; --- a/base/szlibc.c +++ b/base/szlibc.c @@ -110,7 +110,9 @@ s_zlib_free(void *zmem, void *data) gs_free_object(mem, data, "s_zlib_free(data)"); for (; ; block = block->next) { if (block == 0) { +#ifdef DEBUG lprintf1("Freeing unrecorded data "PRI_INTPTR"!\n", (intptr_t)data); +#endif return; } if (block->data == data) --- a/devices/gdevupd.c +++ b/devices/gdevupd.c @@ -1040,8 +1040,13 @@ upd_print_page(gx_device_printer *pdev, */ if(!upd || B_OK4GO != (upd->flags & (B_OK4GO | B_ERROR))) { #if UPD_MESSAGES & (UPD_M_ERROR | UPD_M_TOPCALLS) +#ifdef DEBUG errprintf(pdev->memory, "CALL-REJECTED upd_print_page(" PRI_INTPTR "," PRI_INTPTR ")\n", (intptr_t)udev,(intptr_t) out); +#else + errprintf(pdev->memory, "CALL-REJECTED upd_print_page\n", + (intptr_t)udev,(intptr_t) out); +#endif #endif return_error(gs_error_undefined); } --- a/devices/vector/gdevpdtb.c +++ b/devices/vector/gdevpdtb.c @@ -371,7 +371,7 @@ pdf_base_font_alloc(gx_device_pdf *pdev, font_name.size -= SUBSET_PREFIX_SIZE; } } else { - gs_snprintf(fnbuf, sizeof(fnbuf), ".F" PRI_INTPTR, (intptr_t)copied); + gs_snprintf(fnbuf, sizeof(fnbuf), "Anonymous"); font_name.data = (byte *)fnbuf; font_name.size = strlen(fnbuf); } --- a/psi/ialloc.c +++ b/psi/ialloc.c @@ -386,7 +386,7 @@ gs_free_ref_array(gs_ref_memory_t * mem, size = num_refs * sizeof(ref); break; default: - lprintf3("Unknown type 0x%x in free_ref_array(%u,"PRI_INTPTR")!", + if_debug3('A', "Unknown type 0x%x in free_ref_array(%u,"PRI_INTPTR")!", r_type(parr), num_refs, (intptr_t)obj); return; } --- a/psi/igc.c +++ b/psi/igc.c @@ -1062,7 +1062,7 @@ gc_extend_stack(gc_mark_stack * pms, gc_ if (cp == 0) { /* We were tracing outside collectible */ /* storage. This can't happen. */ - lprintf1("mark stack overflowed while outside collectible space at "PRI_INTPTR"!\n", + if_debug1('6', "mark stack overflowed while outside collectible space at "PRI_INTPTR"!\n", (intptr_t)cptr); gs_abort(pstate->heap); } @@ -1291,7 +1291,7 @@ igc_reloc_struct_ptr(const void /*obj_he if (cp != 0 && cp->cbase <= (byte *)obj && (byte *)obj ctop) { if (back > (cp->ctop - cp->cbase) >> obj_back_shift) { - lprintf2("Invalid back pointer %u at "PRI_INTPTR"!\n", + if_debug2('6', "Invalid back pointer %u at "PRI_INTPTR"!\n", back, (intptr_t)obj); gs_abort(NULL); } --- a/psi/igcstr.c +++ b/psi/igcstr.c @@ -152,7 +152,7 @@ gc_string_mark(const byte * ptr, uint si return false; #ifdef DEBUG if (ptr < cp->ctop) { - lprintf4("String pointer "PRI_INTPTR"[%u] outside ["PRI_INTPTR".."PRI_INTPTR")\n", + if_debug4('6', "String pointer "PRI_INTPTR"[%u] outside ["PRI_INTPTR".."PRI_INTPTR")\n", (intptr_t)ptr, size, (intptr_t)cp->ctop, (intptr_t)cp->climit); return false; } else if (ptr + size > cp->climit) { /* @@ -171,7 +171,7 @@ gc_string_mark(const byte * ptr, uint si while (ptr == scp->climit && scp->outer != 0) scp = scp->outer; if (ptr + size > scp->climit) { - lprintf4("String pointer "PRI_INTPTR"[%u] outside ["PRI_INTPTR".."PRI_INTPTR")\n", + if_debug4('6', "String pointer "PRI_INTPTR"[%u] outside ["PRI_INTPTR".."PRI_INTPTR")\n", (intptr_t)ptr, size, (intptr_t)scp->ctop, (intptr_t)scp->climit); return false; --- a/psi/iinit.c +++ b/psi/iinit.c @@ -395,8 +395,12 @@ zop_init(i_ctx_t *i_ctx_p) if (def->proc != 0) { code = def->proc(i_ctx_p); if (code < 0) { +#ifdef DEBUG lprintf2("op_init proc "PRI_INTPTR" returned error %d!\n", (intptr_t)def->proc, code); +#else + lprintf("op_init proc returned error !\n"); +#endif return code; } } --- a/psi/imainarg.c +++ b/psi/imainarg.c @@ -229,7 +229,8 @@ gs_main_init_with_args01(gs_main_instanc if (gs_debug[':'] && !have_dumped_args) { int i; - dmprintf1(minst->heap, "%% Args passed to instance "PRI_INTPTR": ", + if (gs_debug_c(gs_debug_flag_init_details)) + dmprintf1(minst->heap, "%% Args passed to instance "PRI_INTPTR": ", (intptr_t)minst); for (i=1; iheap, "%s ", argv[i]); --- a/psi/isave.c +++ b/psi/isave.c @@ -487,7 +487,7 @@ alloc_save_change_in(gs_ref_memory_t *me else if (r_is_struct(pcont)) cp->offset = (byte *) where - (byte *) pcont->value.pstruct; else { - lprintf3("Bad type %u for save! pcont = "PRI_INTPTR", where = "PRI_INTPTR"\n", + if_debug3('u', "Bad type %u for save! pcont = "PRI_INTPTR", where = "PRI_INTPTR"\n", r_type(pcont), (intptr_t) pcont, (intptr_t) where); gs_abort((const gs_memory_t *)mem); } --- a/psi/iutil.c +++ b/psi/iutil.c @@ -537,7 +537,11 @@ other: break; } /* Internal operator, no name. */ +#if DEBUG gs_snprintf(buf, sizeof(buf), "@"PRI_INTPTR, (intptr_t) op->value.opproc); +#else + gs_snprintf(buf, sizeof(buf), "@anonymous_operator", (intptr_t) op->value.opproc); +#endif break; } case t_real: