Pages: [1]
  Print  
Author Topic: Buffer Overflow in 0.7 + 0.7.1  (Read 10175 times)
dmn_clown
Posts a lot
*

Cakes 1
Posts: 1324


« on: September 05, 2007, 06:12:47 AM »

There is a buffer overflow in 0.7 and 0.7.1:

Quote
stitched 0 LoD cracks
...loaded 1038 faces, 0 meshes, 0 trisurfs, 7 flares
trying D:\projects\oa\newsparkness\Cone.TGA...
trying D:\projects\oa\newsparkness\Plane.TGA...
trying E:\projects\oa\newplasrailhit\Sphere.TGA...
trying rocketFlare_001.TGA...
trying E:\projects\oa\newtele\Circle.TGA...
trying teleporterEffect.TGA...
Com_sprintf: overflow of 67 in 64

caused by one of the grenade launcher lod models (or more accurately a FUBAR'd export from Blender).
This has been fixed in the SVN and I highly suggest a 0.7.1-1 release that includes the fixed model.
Logged

fromhell
Administrator
GET A LIFE!
**********

Cakes 35
Posts: 14520



WWW
« Reply #1 on: September 05, 2007, 11:20:32 AM »

fkn yikes
I wonder what exactly is causing this though, so it can be prevented in future model exports

I might replace the 0.7.1 patch archive with this, and add a new fixed 0.7.1-1 in addition for those who already have the patch
Logged

asking when OA3 will be done won't get OA3 done.
Progress of OA3 currently occurs behind closed doors alone

I do not provide technical support either.

new code development on github
dmn_clown
Posts a lot
*

Cakes 1
Posts: 1324


« Reply #2 on: September 05, 2007, 01:42:04 PM »

I don't know why the export scripts did this, I do know that opening the md3's and re-saving them in Misfit Model 3D fixed it.
Logged

beast
Lesser Nub


Cakes 0
Posts: 142



« Reply #3 on: September 05, 2007, 03:09:05 PM »

According to the source code, this string is printed to the console when the generated string is larger than the 'size' that is passed into the call to Com_sprintf. The actual write to the destination buffer, though, is done with Q_strncpyz. This ensures that the write to the destination buffer is safe.

Now, if the generated message had been 'Com_sprintf: overflowed bigbuffer', you would have had a local stack variable that got hosed. (The local stack variable is allocated to 32000 bytes, so it would be a monster string :-)

So, for the most part, this error message should be able to be ignored. The string does not contain as much data as it would/should have and likely would only be a problem if it was a file path or something like that where all of the data is important. (Worse case, a model won't get loaded because the full path didn't make it to the string... Something like that...)

If I read the source correctly, there was no buffer overflow, however...

Hope this helps...

[update]
It appears that it is related to not finding a model/texture/whatever the heck it is. The path to the file that it is 'trying' is too long. When you redid the file(s) in Misfit, the path(s) were probably shorter and the warning went away.

Being a noob, it does make me want to ask... Why are all those paths stored with the files? It seems that all of that data should be removed before the files are released. (Potential private information is being included there...)
[/update]
« Last Edit: September 05, 2007, 03:50:08 PM by beast » Logged
fromhell
Administrator
GET A LIFE!
**********

Cakes 35
Posts: 14520



WWW
« Reply #4 on: September 05, 2007, 04:14:15 PM »

Well, that's because tr3b wanted direct paths in his scripts, i didn't like this, and he said I should do this :/

Including D:\projects and D:\svnthis isn't that killer, though this mostly only happens on player models exported with the old script. Re-exporting them with the newer script fork posted won't jar out some long pathnames.
Logged

asking when OA3 will be done won't get OA3 done.
Progress of OA3 currently occurs behind closed doors alone

I do not provide technical support either.

new code development on github
dmn_clown
Posts a lot
*

Cakes 1
Posts: 1324


« Reply #5 on: September 05, 2007, 06:31:33 PM »

According to the source code, this string is printed to the console when the generated string is larger than the 'size' that is passed into the call to Com_sprintf. The actual write to the destination buffer, though, is done with Q_strncpyz. This ensures that the write to the destination buffer is safe.

Yes, but if I say 'size' overflow no one knows what I am talking about.

Quote
(Worse case, a model won't get loaded because the full path didn't make it to the string... Something like that...)

Not quite, a malformed md3 can be used to inject code and is one of the reasons auto-download is off by default.

Well, that's because tr3b wanted direct paths in his scripts, i didn't like this, and he said I should do this :/

But you don't need direct paths, you only need the path within the VFS (virtual file system) and considering the limitations of the md3 format:
Quote
The MD3 file format has several limits that you should consider when making a model for this format. These are outlined below.
  • Only faces and verticies in a group get saved.
  • The MD3_PATH+file name cannot be longer than 63 characters.  (Textures and model names)
  • The group name and point name cannot be longer than 63 characters.
  • Maximum of 1024 animation frames
  • Maximum of 16 Points (Quake 3 Tags)
  • Maximum of 32 Groups
  • Maximum of 256 Textures per Group
  • Maximum of 8192 Triangles per Group
  • Maximum of 4096 Verticies per Group
It's not a good idea to include more information than you absolutely need.
« Last Edit: September 05, 2007, 07:00:25 PM by dmn_clown » Logged

beast
Lesser Nub


Cakes 0
Posts: 142



« Reply #6 on: September 06, 2007, 12:28:22 AM »

Not quite, a malformed md3 can be used to inject code and is one of the reasons auto-download is off by default.
Agreed...

Quote
  • The MD3_PATH+file name cannot be longer than 63 characters.  (Textures and model names)
The 63 character limit for the MD3_PATH+filename was what was getting hit. The line that generates this message  on my machine is:

D:\svnthis\oa2\source\assets\models\newgren\progs/v_shot_v_sHOT.TGA

The last 4 characters were clipped off by the Q_strncpyz function. So it came out as:

D:\svnthis\oa2\source\assets\models\newgren\progs/v_shot_v_sHOT

So, in this case, there was not a buffer overflow- there would have been if Com_sprintf/Q_strncpyz  had not done its job.  The path was incomplete and the file was not found.  ( Of course, the file would not have been found on my machine even if the entire path was in the string, but that's a different part of the story... :-)
Logged
dmn_clown
Posts a lot
*

Cakes 1
Posts: 1324


« Reply #7 on: September 06, 2007, 05:12:31 AM »

The 63 character limit for the MD3_PATH+filename was what was getting hit.

I know that now (it hit me just after I posted the limits), and it is all the more reason not to use direct paths.  It is a design flaw in the xreal md3 export scripts (also in fork's) which have no limitation on the number of characters you can bury in a model's meta-data.
« Last Edit: September 06, 2007, 06:22:03 AM by dmn_clown » Logged

Pages: [1]
  Print  
 
Jump to: