3

We are using DevPartners boundchecker for detecting memory leak issues. It is doing a wonderful job, though it does not find string overflows like the following

char szTest [1] = "";

for (i = 0; i < 100; i ++) {

    strcat (szTest, "hi");
}

Question-1: Is their any way, I can make BoundsChecker to detect this?

Question-2: Is their any other tool that can detect such issues?

Tanzelax
  • 5,406
  • 2
  • 25
  • 28
Alphaneo
  • 12,079
  • 22
  • 71
  • 89

8 Answers8

2

One option is to simply ban the use of string functions that don't have information about the destination buffer. A set of macros like the following in a universally included header can be helpful:

#define strcpy  strcpy_is_banned_use_strlcpy
#define strcat  strcat_is_banned_use_strlcat
#define strncpy strncpy_is_banned_use_strlcpy
#define strncat strncat_is_banned_use_strlcat
#define sprintf sprintf_is_banned_use_snprintf

So any attempted uses of the 'banned' routines will result in a linker error that also tells you what you should use instead. MSVC has done something similar that can be controlled using macros like _CRT_SECURE_NO_DEPRECATE.

The drawback to this technique is that if you have a large set of existing code, it can be a huge chore to get things moved over to using the new, safer routines. It can drive you crazy until you've gotten rid of the functions considered dangerous.

Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • 1
    @Michael - while I like this idea, one issue is that you cannot always replace calls to strnX with calls to strlX. The problem is that the strn functions can be used when the source is not 0 terminated but the strl functions cannot handle that case. – R Samuel Klatchko Feb 24 '10 at 01:41
  • I agree that's it's not a complete solution, but overall it's worked well for some projects I've worked on. The few times that the `strlX()` functions couldn't be used there were other alternatives (usually `memcpy()`) that could be used instead. And the `strlX()` functions aren't perfect (I've grown to hate the fact that the destination buffer size isn't immediately after the destination pointer), but they're generally better than the functions that have no idea what the size of the destination is. And if you don't like `strlX()` it's not hard to come up with your own (like MSVC did). – Michael Burr Feb 24 '10 at 01:50
  • If you know the buffer size and it isn't ludicrously huge, a fixed length memcpy can be three times faster than any str* function. – Zan Lynx Feb 24 '10 at 02:07
2

valgrind will detect writing past dynamically allocated data, but I don't think it can do so for automatic arrays like in your example. If you are using strcat, strcpy, etc., you have to make sure that the destination is big enough.

Edit: I was right about valgrind, but there is some hope:

Unfortunately, Memcheck doesn't do bounds checking on static or stack arrays. We'd like to, but it's just not possible to do in a reasonable way that fits with how Memcheck works. Sorry.

However, the experimental tool Ptrcheck can detect errors like this. Run Valgrind with the --tool=exp-ptrcheck option to try it, but beware that it is not as robust as Memcheck.

I haven't used Ptrcheck.

Alok Singhal
  • 93,253
  • 21
  • 125
  • 158
2

I tried it in my devpartner (msvc6.6) (devpartner 7.2.0.372)

I confirm your observed behavior. I get an access violation after about 63 passes of the loop.

What does compuware have to say about the issue?

CppCheck will detect this issue.

EvilTeach
  • 28,120
  • 21
  • 85
  • 141
1

You may find that your compiler can help. For example, in Visual Studio 2008, check the project properties - C/C++ - Code Generation page. Theres a "Buffer Security Check" option.

My guess would be that it reserves a bit of extra memory and writes a known sequence in there. If that sequence gets modified, it assumes a buffer overrun. I'm not sure, though - I remember reading this somewhere, but I don't remember for certain if it was about VC++.

1

Given that you've tagged this C++, why use a pointer to char at all?

std::stringstream test;
std::fill_n(std::ostream_iterator<std::string>(test), 100, "hi");
Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • STL isnt portable on all platform – YeenFei Feb 24 '10 at 01:39
  • 1
    @YeenFei:it's not at all clear what you mean by "STL". What I have above is a required part of the standard library. If your compiler has such a lousy standard library that this simple code doesn't work, you need to find a new one (Comeau does custom ports of their compiler). – Jerry Coffin Feb 24 '10 at 02:05
  • 1
    @Jerry - it might be less "lousy" and more old. e.g. VC++ 6 is still in use in some corners of the world (maybe for interop with VB 6, maybe in a part of the world that can't afford to upgrade everything every other day, etc etc). Also, he may not be worried about "his compiler". Maybe he's selling to the guys who are still using VC++ 6? –  Feb 24 '10 at 03:04
  • 1
    @Steve314: VC++ 6 will handle that code without any problems at all though. I'm pretty sure to get that to fail, you'd have to go back to something like VC++ 4.1 or so -- by VC++ 4.2b I was doing simple STL things like this pretty regularly. – Jerry Coffin Feb 24 '10 at 04:10
  • I used the STL in VC++ 6 - and I remember it failing. I won't place bets, but I don't think it even had stringstream - it *did* have ostrstream and istrstream, but that's not the same. –  Feb 24 '10 at 04:43
  • 2
    @Steve314:yes, it most assuredly did have stringstream. It certainly had some bugs (especially early on, but six service packs helped a lot). I've used it enough, recently enough, to be quite certain the code above would work just fine with it. – Jerry Coffin Feb 24 '10 at 05:04
1

If you enable the /RTCs compiler switch, it may help catch problems like this. With this switch on, the test caused an access violation when running the strcat only one time.

Another useful utility that helps with problems like this (more heap-oriented than stack but extremely helpful) is application verifier. It is free and can catch a lot of problems related to heap overflow.

Mark Wilkins
  • 40,729
  • 5
  • 57
  • 110
  • @Mark Wilkins: Thank you for your answer. I am using MSVC 6.0; and I enabled the RTCs flag, and it did not detect the error. Anyway, I will try in this direction. Thank you. – Alphaneo Mar 01 '10 at 00:20
0

An alternative: our Memory Safety Checker. I think it will handle this case.

Ira Baxter
  • 93,541
  • 22
  • 172
  • 341
0

The problem was that by default, the API Validation subsystem is not enabled, and the messages you were interested in come from there.

I can't speak for older versions of BoundsChecker, but version 10.5 has no particular problems with this test. It reports the correct results and BoundsChecker itself does not crash. The test application does, however, because this particular test case completely corrupts the call stack that led to the function where the test code was, and as soon as that function terminated, the application did too.

The results: 100 messages about write overrun to a local variable, and 99 messages about the destination string not being null terminated. Technically, that second message is not right, but BoundsChecker only searches for the null termination within the bounds of the destination string itself, and after the first strcat call, it no longer contains a zero byte within its bounds.

Disclaimer: I work for MicroFocus as a developer working on BoundsChecker.

Rick Papo
  • 369
  • 1
  • 2
  • 10