bugvile, a vi-compatible text editor - Bugs: bug #65965, filters.c :: alloc_keyword() can...

 
 

bug #65965: filters.c :: alloc_keyword() can leave unalloc'ed memory pointer dangling

Submitter:  None
Submitted:  Mon 08 Jul 2024 09:09:01 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Sat 08 Feb 2025 01:29:21 AM UTC, comment #2: 

I made a fix for this in 9.8za

On my to-do list I also had a note about a possible memory leak when using "-i" (case-independent) matching as in the Ada filter, so I ran some tests with valgrind and found no problems.

Thomas Dickey <dickey>
Group administrator
Tue 09 Jul 2024 12:37:22 AM UTC, comment #1: 

It would help to have a testcase which demonstrates the bug.  I noticed some potential for memory-leaks via get_keyword_attr when the filter does a "-i" option, but haven't noticed a case where filters are broken (recently...)

Thomas Dickey <dickey>
Group administrator
Mon 08 Jul 2024 09:09:01 PM UTC, original submission:  

Looking inside filters/filters.c in 9.8z :

Function alloc_keyword() first looks for 'ident'; if already exists, attempts to overwrite its attribute by strmalloc()ing a replacement.

However if the strmalloc() fails [granted, you're pretty much screwed anyway if you're OOM at this point haha], it frees 'nxt' i.e. it frees the RETURNED VALUE of FindIdentifier().

...Which in turn...(if you're not USE_TSEARCH-ing)...was...a piece of the my_table[] array (either my_table[i] for some 'i', or my_table[i]->kw_next(->kw_next...) i.e. some 'kw_next' member of the limb.

However that now-freed pointer-value of 'nxt' is still alive within the my_table[] array from whence it was fetched !  (and no obvious way of zapping it as we've left FindIdentifier() ).

I suggest redoing the logic such that if the strmalloc() fails, the existing value is kept.  I.e. alloc the strmalloc first but don't assign to nxt->kw_attr unless it succeeded.  It'll just use the previous attribute...

Adam Denton

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dickey (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-02-08 dickey StatusNone Fixed

    Back to the top

    Powered by Savane 3.15-26b0.
    Corresponding source code