ITS4 from Cigital flawed

From: David LeBlanc (dleblancat_private)
Date: Mon Feb 18 2002 - 03:19:15 PST

  • Next message: David LeBlanc: "RE: In response to alleged vulnerabilities in Microsoft Visual C++ security checks feature"

    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 <windows.h>
    
    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
    dleblancat_private 
    



    This archive was generated by hypermail 2b30 : Tue Feb 19 2002 - 12:02:53 PST