ansaurus

Question

Answer 1

+3 A:

How long is the directory name, and how long is the buffer you're trying to store it in? You haven't given us much to go on...how about showing some code? Perhaps a call to sprintf somewhere in main(), and the declarations of any variables involved?

Edit: It sure looks like filename needs to be a bigger array, given your input directory and the filename you're appending to it! Quick fix: try declaring it as, say, 1500 characters instead of 50. Better fix: since you're using C++, look into the std::string and ostringstream classes, which will resize themselves to prevent buffer overflows.

To answer your followup questions:

The "../" in your result-path shouldn't get expanded into an absolute path.

My hunch that sprintf() was involved was a lucky guess, based on the "buffer overflow" message and the last few lines in the gdb traceback. I'm not that familiar with glibc internals, but perhaps __sprintf_chk() and __vsprintf_chk() are buffer-overflow-checking variants of sprintf()?

Jim Lewis 2009-09-03 00:01:10

Thank you! Please see my update at the end of the original post.

Tim 2009-09-03 00:31:13

Thanks! A little curious though, why without optimization the executable runs fine, no reporting buffer overflow?

Tim 2009-09-03 01:00:09

@Tim: This bug results in what's known as "undefined behavior" -- meaning: it may crash, it may work fine, it may cause demons to come flying out of your nose. Probably, the compiler has rearranged the layout of data in the optimized version, so when you overrun the buffer it clobbers something else, causing the error message you saw.

Jim Lewis 2009-09-03 01:21:48

Answer 2

+1 A:

Well you use sprintf to print to the buffer 'filename[50]' which of course has length 50. Now the string you are printing is a buffer of size 1024, this strikes me as a potential issue. What happens when global.result_path is longer than 50 (even less actually, as you are also printing integers), well you get an overflow.

Try using C++ std::string and std::stringstream, ie:

//Part 1:

std::stringstream ss;
ss << global.result_path << /* other data */;
train_samples[n].write_png(ss.str().c_str());

//Part 2:

class Global
{
    std::string result_path;
    ...
}

With the above code you will never have to worry about overflowing character buffers or any of that other ugly stuff.

DeusAduro 2009-09-03 00:49:04

Answer 3

+2 A:

I long ago got into the habit of using snprintf everywhere. Learn to love it. It may still fail to write the correct file, but at least it won't leave a security hole.

Then after you start to wonder why your program is creating files named "this_is_a_long_file_na" you can go back and fix it to either use a buffer of PATH_MAX or a dynamic sized malloc'd buffer. snprintf will help you find the right size if the buffer needed to be bigger.

Or you can switch to C++ and use std::string.

Zan Lynx 2009-09-03 02:01:04

+1 for the PATH_MAX reference and mentioning that dynamic allocation or std::string would definitely be better

Matthew Iselin 2009-09-03 02:14:50

Answer 4

+1 A:

result_path is too small.

Simply change result_path to 1024. Some systems have the macro MAX_PATH defined. I'd also change sprintf to snprintf with the size as sizeof(result_path).

The snprintf() function is just like sprintf(), except that the length of the buffer is given. This prevents buffer overflows.

The return value is the number of characters written. If the output was truncated due to buff_size limit then the return value is the number of characters (not including the trailing '\0') which would have been written to the final string if enough space had been available.

How I knew you had a problem with sprintf was the backtrace.

i.e.

(gdb) bt

#0 0x00007fbd29573fb5 in raise () from /lib/libc.so.6

#1 0x00007fbd29575bc3 in abort () from /lib/libc.so.6

#2 0x00007fbd295b3228 in ?? () from /lib/libc.so.6

#3 0x00007fbd296402c7 in __fortify_fail () from /lib/libc.so.6

#4 0x00007fbd2963e170 in __chk_fail () from /lib/libc.so.6

#5 0x00007fbd2963d519 in ?? () from /lib/libc.so.6

#6 0x00007fbd295b7426 in _IO_default_xsputn () from /lib/libc.so.6

#7 0x00007fbd29586fdb in vfprintf () from /lib/libc.so.6

#8 0x00007fbd2963d5b9 in __vsprintf_chk () from /lib/libc.so.6

#9 0x00007fbd2963d500 in __sprintf_chk () from /lib/libc.so.6

#10 0x0000000000408695 in main ()

i.e. You have a function main in your code. and __sprintf_chk is where it goes belly up. You had to be calling sprintf. After that it died. So my guess was that you were passing it bad arguments. The only way sprintf can die so badly is with a buffer overflow. So it's a good assumption that the string you're printing to is too small. Use snprintf and it'll be much safer. You can then print to debug the result. If you'd done that you would have seen straight away that the buffer was too small as the result_path would have been truncated at 50 chars and the program wouldn't have crashed (at that point at least :).

Matt H 2009-09-03 02:38:25

ansaurus

tags:

views:

answers:

writing image to directory fail

related questions