Bug 210829 - databases/db5: db-5.3.28/src/heap/heap_verify.c can pass __os_free(dbp->env, offsets) an uninitialized offsets value (a bad pointer)
Summary: databases/db5: db-5.3.28/src/heap/heap_verify.c can pass __os_free(dbp->env, ...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Matthias Andree
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-04 21:27 UTC by Mark Millard
Modified: 2016-11-09 22:34 UTC (History)
0 users

See Also:
mandree: maintainer-feedback+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Millard 2016-07-04 21:27:44 UTC
db-5.3.28/src/heap/heap_verify.c has code of the structure:

int
__heap_vrfy(dbp, vdp, h, pgno, flags)
        DB *dbp;
        VRFY_DBINFO *vdp;
        PAGE *h;
        db_pgno_t pgno;
        u_int32_t flags;
{
        HEAPHDR *hdr;
        int cnt, i, j, ret;
        db_indx_t *offsets, *offtbl, end;
 
        if ((ret = __db_vrfy_datapage(dbp, vdp, h, pgno, flags)) != 0)
                goto err;
. . .
 err:   __os_free(dbp->env, offsets);
        return (ret);
}

If the listed goto is executed then __os_free is passed an uninitialized offsets value (a junk pointer).

This was reported by the compiler used to build databases/db5.
Comment 1 Matthias Andree freebsd_committer freebsd_triage 2016-07-06 08:07:29 UTC
What do you propose we do about this?
Comment 2 Mark Millard 2016-07-06 08:46:45 UTC
(In reply to Matthias Andree from comment #1)

I was not directly trying to build db5. It just showed up via dependencies. So I'm not familiar with it at all.

Since bad pointers and freeing them tend to be security risks as well as just wrong so I'd hope for changes to avoid the bad (uninitialized) pointer use.

Looking at db-5.3.28/src/os/os_alloc.c and its __os_free(. . .) implementation (found via grep) indicates that the such is an issue here:

void
__os_free(env, ptr)
        ENV *env;
        void *ptr;
{
. . .
        if (DB_GLOBAL(j_free) != NULL)
                DB_GLOBAL(j_free)(ptr);
        else
                free(ptr);
}

I will not show the whole routine here but all code paths require ptr to have a valid pointer value for testing and/or use.

My quick estimate would be that the specific "goto err" that I originally showed should instead be "return ret;" (or "return (ret);" in the style of the existing code).

If you have upstream contacts it would seem appropriate to propagate the information upstream (where ever that is).
Comment 3 Mark Millard 2016-08-02 08:42:43 UTC
The build was of db5-5.3.28_4 --which is still in place as of /usr/ports/ -r419343 .
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-11-09 22:32:02 UTC
A commit references this bug:

Author: mandree
Date: Wed Nov  9 22:31:49 UTC 2016
New revision: 425813
URL: https://svnweb.freebsd.org/changeset/ports/425813

Log:
  Avoid junk pointer when __db_vrfy_datapage() fails

  Rather than second-guessing what the __os_free() might be doing and
  avoiding it, initialize the pointer to NULL, which __os_free() will
  skip.  This should be the safer approach if Oracle ever patches other
  parts of db 5.3.

  PR:		210829
  Submitted by:	Mark Millard

Changes:
  head/databases/db5/Makefile
  head/databases/db5/files/patch-lang_tcl_tcl__env.c
  head/databases/db5/files/patch-lang_tcl_tcl__seq.c
  head/databases/db5/files/patch-src_heap_heap__verify.c
Comment 5 Matthias Andree freebsd_committer freebsd_triage 2016-11-09 22:32:52 UTC
I don't have upstream contacts, nor have I found a bug report address on Oracle's website that wouldn't require logins or contracts.  If I've missed one, drop me a heads-up or someone file this with Oracle and add a comment to this PR.
Comment 6 Matthias Andree freebsd_committer freebsd_triage 2016-11-09 22:34:33 UTC
The commit accidentally bumped files/patch-lang_tcl_tcl__env.c and files/patch-lang_tcl_tcl__seq.c. These changes only affected patch/diff timestamps but not content.