wichr / eepe

Automatically exported from code.google.com/p/eepe
0 stars 0 forks source link

Buffer overruns in sevreral places, exposed on Fedora 14 stock #105

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Compile eepe with Fedora 14, stock Qt, stock compiler, latest updates
2. run eepe, try to load older eepe file that worked a week ago OR
   file->new, then try to edit any blank model.  Or edit a model, and make it's name the maximum number of characters (10) and close the window.
3. instant crash

What is the expected output? What do you see instead?
I see a crash, always from glibc's __stack_chk_fail().

What version of the product are you using? On what operating system?
Latest SVN rev (r262), Fedora 14 32-bit, Qt 4.7.3

Please provide any additional information below.

modeledit.cpp line 1405 overruns in strcpy if the contents of the model name 
field have 10 characters (the max).  strcpy is always dangerous.  Would strncpy 
work here?  I know there are issues with strncpy and trailing nulls if the 
buffer is too small.

mdichild.cpp line 149, buf overruns somewhere in MdiChild::refreshList()

I think that these buffer overruns have already existed, and Fedora is now 
making them fatal errors, instead of silent errors (see line 1401 in 
modeledit.cpp as a symptom of this!)

I'm not familiar enough with how the data goes from the C++ structures to the 
eeprom file, but it seems to me that t_ModelData.name is too short by one 
character, at least if strcpy is to be used.  If we use strncpy, and then in 
places that access the .name field, limit it to 10 bytes, that would work.  I 
will explore this and attach a patch for review.

Original issue reported on code.google.com by torr...@gmail.com on 5 Aug 2011 at 9:01

GoogleCodeExporter commented 8 years ago
So I found the reason these errors are only hitting now.  Just a week ago I 
installed an update to qt-devel (qt-4.7.3-6.fc14.src.rpm) that must now default 
to -fstack-protector being a compile flag now.  Anyway, I'll take a stab at 
finding the overruns and fixing them.

Original comment by torr...@gmail.com on 6 Aug 2011 at 2:50

GoogleCodeExporter commented 8 years ago
Here's a patch that fixes the buffer overruns that were causing glibc to abort 
eepe.  The fix in mdichild.cpp should be okay as it looks like buf is long 
enough to handle anything, so shortening the buf by one byte when converting to 
ascii will work.  I though that giving QString::fromAscii a length of 20 would 
work, but it still crashes, so I had to give it 19 instead.  

Original comment by torr...@gmail.com on 7 Aug 2011 at 4:00

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks, I'll add it.

Original comment by erezra...@gmail.com on 7 Aug 2011 at 4:15

GoogleCodeExporter commented 8 years ago
Seems to work fine.   I guess I live a sheltered life working on Mint/Ubuntu.
I'll close this issue but feel free to keep on posting either here or to other 
issues. 

Original comment by erezra...@gmail.com on 7 Aug 2011 at 4:18

GoogleCodeExporter commented 8 years ago
So just for your information, if you add "-fstack-protector" to the compile 
flags (and maybe -D_FORTIFY_SOURCE=2), then Qt should abort (via glibc) when a 
buffer is overrun.  This is how Qt is configured on Fedora.

Original comment by torr...@gmail.com on 7 Aug 2011 at 5:13

GoogleCodeExporter commented 8 years ago
Did this patch get applied?  I updated the source code to r271 and the changes 
aren't there. 

Original comment by torr...@gmail.com on 29 Sep 2011 at 11:58