From: David LeBlanc (dleblanc@mindspring.com)
To: bugtraq@securityfocus.com
Date: Mon, 18 Feb 2002 03:19:15 -0800
Subject: ITS4 from Cigital flawed

Cigital's web site says:

"When it comes to software security, there's no such thing as a small
coding error. A single mistake in a single line of code can leave your
business-critical applications vulnerable to serious security breaches."

And then it offers this neat little code scanning tool called ITS4. I
decided to check it out. It's web page said:

----------------------------------
ITS4
Software Security Tool
The Software Security Group at Cigital designs, analyzes and tests
security-critical software. We developed ITS4 to help automate source
code review for security. ITS4 is a simple tool that statically scans C
and C++ source code for potential security vulnerabilities. It is a
command-line tool that works across Unix and Windows platforms.

ITS4 scans source code, looking for function calls that are potentially
dangerous. For some calls, ITS4 tries to perform some code analysis to
determine how risky the call is. In each case, ITS4 provides a problem
report, including a short description of the potential problem and
suggestions on how to fix the code.
-----------------------------------

Sounds great. Let's test it - 

Here's the code I threw at it:

-------- start test.c ------------------
#include 

int doh(char* buf, char* result, int num)
{
  strncpy(result, buf, num);
  
}

int main(int argc, char* argv[])
{
  char buf[10];
  WCHAR* w = L"String That is too long for buf";
  SOCKET s;

  lstrcpy(buf, argv[1]); //overflow not detected
  strcpy(buf, argv[1]); //correct

  _ultoa(0xffffffff, buf, 10); //whups - won't find this, either

  //overflow not detected
  sprintf(buf,
"fooooooooooooooooooooooooooooooooooooooooooooooooooooo"); 
  
  //correct warning about format string, no warning about overflow
  sprintf(buf, argv[1]); 

  //overflow not detected - wrong size arg - big mistake
  WideCharToMultiByte(CP_ACP, 0, w, wcslen(w), buf, 20, NULL, NULL); 

  doh(buf, "Long string that won't fit", 256); //doesn't catch this,
either

  recv(s, buf, 256, 0); //doesn't get this one, either
  return 0;
}
--------- end test.c ----------------------

As you can see from the comments, it doesn't seem to work very well.
Here's the actual output:

[e:\projects\its4]its4.exe -v vulns.i4d test.c
test.c:17:(Urgent) sprintf
Non-constant format strings can often be attacked.
Use a constant format string.
----------------
test.c:14:(Very Risky) strcpy
This function is high risk for buffer overflows
Use strncpy instead.
----------------

I find it kind of interesting that the /GS option (or StackGuard) would
have kept each and every example overflow from being exploitable. It
doesn't seem to know about some very commonly used Windows variants of
strcpy. This isn't good. It also doesn't seem to help with feeding even
a very long static string into an insufficient buffer using sprintf
(which is really the simplest case for sprintf). As sprintf is
cross-platform, this is a more serious issue. Even passing a format
string directly doesn't trigger a warning of a buffer overflow.
Overflowing a buffer with ultoa doesn't get caught, either. Next,
WideCharToMultiByte doesn't trigger a warning - this is exactly the sort
of thing that caused the .ida overflow, and a common mistake to make
with this function. Finally, it doesn't take much analysis to follow
arguments into a trivial function like doh() - it misses this one, too.
This is all I found in half an hour. I'm sure I could probably find more
if I looked.

Perhaps it can be improved in the future. As Gary said, if you insist on
using Cigital's ITS4, at least know the risks!

IMHO, I'm going to skip using this one - maybe next version will do
better. I'd thought it may not do too well with Win32-specific problems,
but it doesn't seem very effective at regular cross-platform code,
either. Out of 9 problems in the code, it only flagged 2 of them.

David LeBlanc
dleblanc@mindspring.com