Re: Reporting Security Violations in Software Package

[Thomas Dickey]

On Wed, Mar 12, 2025 at 07:24:20PM -0400, Bill Gray wrote:
> On 3/12/25 14:57, Thomas Dickey wrote:
> > There's nothing new here - read the FAQ
> > 
> > https://invisible-island.net/ncurses/ncurses.faq.html#config_leaks
> 
>    I may be misunderstanding the report,  but it looks to me as if it
> involves some memory leaks (really,  just unfreed memory allocations),
> followed by some possibly more troublesome issues.  For example,  the
> unsigned integer overflow (maybe really 'underflow') at
> 
> tinfo/comp_scan.c:285:26
> 
>  } while (bufptr[len - 1] != '\n');   /* complete a line */
> 
>    ...where I don't see,  offhand,  proof that you won't hit len=0.

maybe.  The input is (supposed to) contain no nulls because of a change
I made a couple of years ago, so that len should always be at least 1.

So I asked for a test-case to demonstrate the problem.  The warning doesn't
tell me much:

../../ncurses/tinfo/comp_scan.c:285:15: runtime error: addition of unsigned 
offset to 0x619000000082 overflowed to 0x619000000081

i.e., if it's really got to this value, I'd expect a _lot_ of diagnostics:

        107,271,103,185,025 

The case on line 142 could be something like this:

        len = 0
        while (len--) {
        }

which makes the loop never executed, but eliciting a warning from a tool.
The listings look like valgrind, which is pretty accurate for C.

I pick away at the cases I can verify, which leaves several which are
compiler bugs.  If there were fewer of those, my list would be easier
to prune.

>    A possibly helpful modification to that "testing for memory leaks" URL :
> as you note,  if you use initscr() and endwin(),  you can't free the memory
> associated with the screen.  However,  if you use
> 
>    SCREEN *screen_ptr;
> 
>    screen_ptr = newterm( NULL, stdout, stdin);
>    ...
>    endwin( );
>    delscreen( screen_pointer);

agreed - there are things that improve the result, but the discussion
usually moves away from user-supplied improvements.  hence the FAQ.

But most applications use initscr...
 
>    then ncurses has only six or seven dangling unfreed allocations. That's
> nowhere near the number of tiny little allocations left over if you just
> call endwin().  I've found this to be a major help when using Valgrind with
> ncurses.
> 
>    The above also has the added benefit of being portable among all
> implementations of curses,  instead of being ncurses-specific.
> 
>    (Even those remaining bits _could_ be cleaned up if desired when the last
> screen was freed.  I'd think that might be tempting;  it would eliminate the
> need for a separate debug version.)

maybe - I mostly notice the ones in error-exits
 
> -- Bill
> 
> > ----- Original Message -----
> > | From: "Yao Shuangjie" <sjyao2022@outlook.com>
> > | To: "Ncurses Mailing List" <bug-ncurses@gnu.org>
> > | Sent: Wednesday, March 12, 2025 10:59:07 AM
> > | Subject: Reporting Security Violations in Software Package
> > 
> > | Dear maintainers,
> > |
> > | We are cybersecurity researchers from the Hong Kong University of Science 
> > and
> > | Technology. We found several security violations of undefined behaviors 
> > and
> > | memory leaks in GNU ncurses 6.5 using our novel symbolic execution 
> > technique
> > | several months ago. The details are shown below.
> > 
> > ...if you want to report a bug, a test-case is required.
> > 
> 
> 

-- 
Thomas E. Dickey <dickey@invisible-island.net>
https://invisible-island.net

signature.asc
Description: PGP signature