buglibiconv - Bugs: bug #58903, gentranslit has a possible...

 
 

bug #58903: gentranslit has a possible buffer-overflow

Submitter:  Reini Urban <reini_urban>
Submitted:  Wed 05 Aug 2020 09:00:48 AM UTC
   
 
Category:  Build Severity:  3 - Normal
Item Group:  None Status:  Fix Released
Privacy:  Public Assigned to:  haible
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 07 Aug 2020 01:11:42 AM UTC, comment #5: 

I reproduce the warning with "gcc -Wall" (GCC >= 7).

Fixed through this commit:
https://git.savannah.gnu.org/gitweb/?p=libiconv.git;a=commitdiff;h=18b2324bc58d26109da63bc599c515dfc5c2995d

Thanks for the report!

Bruno Haible <haible>
Group administrator
Wed 05 Aug 2020 08:03:05 PM UTC, comment #4: 

oops, you are right, preinc.
Attached is the revised patch #2.

Reini Urban <reini_urban>
Wed 05 Aug 2020 07:25:51 PM UTC, comment #3: 


> the malloc reserves 2 for this, so the assert extends it up to 99


Note that technically, the assert as written does not guard against printing 100 in:


   assert (i < 100);
   sprintf(s, "%02x_%d", p, ++i);


I would also question the wisdom of using assert(), which can be disabled at pre-processing, for a safety check, but that could be argued to be a question of style.

Pascal Cuoq <pascal_cuoq>
Wed 05 Aug 2020 05:08:55 PM UTC, comment #2: 

comment #1:

> Could you please explain what symptoms you have been seeing?
> Just a warning from gcc, when you enabled non-default warnings?


The gcc warning with default compilation from git with fedora 31/gcc 9

> Or a crash (and if so, under which circumstances)?
> Or have you been adding lots of stuff to translit.def?


No.

> The largest value 'i' attains, with the translit.def in the distribution, is 5. Two digits are already more than enough for a number in the range [1, 5].


Yes, I'm aware of that, but the malloc reserves 2 for this, so the assert extends it up to 99. It matches the code.

Reini Urban <reini_urban>
Wed 05 Aug 2020 01:30:36 PM UTC, comment #1: 

Could you please explain what symptoms you have been seeing?
Just a warning from gcc, when you enabled non-default warnings?
Or a crash (and if so, under which circumstances)?
Or have you been adding lots of stuff to translit.def?

The largest value 'i' attains, with the translit.def in the distribution, is 5. Two digits are already more than enough for a number in the range [1, 5].

Bruno Haible <haible>
Group administrator
Wed 05 Aug 2020 09:00:48 AM UTC, original submission:  

we reserve only 2 byte for a %d format.
Detected by gcc 9 -W-Wformat-overflow

See patch

Reini Urban <reini_urban>

 

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

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pascal_cuoq (Posted a comment)
  • -email is unavailable- added by haible (Posted a comment)
  • -email is unavailable- added by reini_urban (Submitted the item)
  • -email is unavailable- added by reini_urban
  •  

    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.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-04 haible StatusFixed Fix Released
    2020-08-07 haible CategoryTransliteration Build
        StatusNone Fixed
        Assigned toNone haible
        Open/ClosedOpen Closed
        Summaryassert gentranslit buffer-overflow gentranslit has a possible buffer-overflow
    2020-08-05 reini_urban Attached File- Added 0001-assert-gentranslit-buffer-overflow-2.patch, #49637
    2020-08-05 reini_urban Attached File- Added 0001-assert-gentranslit-buffer-overflow.patch, #49634
        Carbon-Copy- Added reini_urban

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code