#103 [providerRegister.c:195]: (error) Dangerous usage of 'fin' (strncpy doesn't always null-terminate it).

1.4.8
fixed
None
None
Function
2014-03-27
2014-02-25
dcb
No

Source code is

strncpy(fin, dir, sizeof(fin)-18); / 18 = strlen("/providerRegister")+1 /
strcat(fin, "/providerRegister");

I suspect something like

strncpy(fin, dir, sizeof(fin)-18); / 18 = strlen("/providerRegister")+1 /
fin[sizeof(fin)-18] = '\0';
strcat(fin, "/providerRegister");

might be better code.

Also, there seems to be a memory leak

[sfcbproc.c:500]: (error) Resource leak: fp

Suggest add missing call to fclose.

I found both these bugs by using cppcheck, a static analysis tool.

Discussion

  • Chris Buccella

    Chris Buccella - 2014-02-25
    • assigned_to: Dave Heller
     
  • Dave Heller

    Dave Heller - 2014-02-26

    I think the tool is complaining about the rather ugly behavior of strncpy(), which does not set a trailing null when the length of the 'from' string is > 'size'. It does this, I suppose, to preserve as much of the 'from' string as possible in the case where the data needs to be truncated. But it does it at the expense of guaranteeing a safe, null-terminated string.

    This is a well known (but often overlooked!) behavior of strncpy().

    Your patch works, but for consistency I would prefer to reuse a function strncpy_kind() that I recently added to control.c for just for this purpose. Proposed patch attached.

    I am curious to know if this silences the cppcheck warning. I was not able to make cppcheck produce that warning to begin with, but I'm not very familiar with this tool. If you could post the cppcheck cmdline you are using, that would be helpful.

    As for sfcbproc... yes, there are few leaks, etc. that I can also see with cppcheck. But this is just a utility program, short running, so leaks are not really important there, so I'm going to leave that alone for now.

    Thanks for reporting this and getting me looking at cppcheck.

     
  • dcb

    dcb - 2014-02-26

    If you could post the cppcheck cmdline you are using

    Flag --enable=all on the command line makes sure that cppcheck
    looks for everything it knows about.

    Without this flag, i.e. by default, it doesn't say very much.

    There is also an intermediate flag, --enable=warning, which
    is part way between the silence of no flag and the full-on
    super fussy --enable=all flag.

     
  • Dave Heller

    Dave Heller - 2014-02-26

    Yes, I see now. and this does silence the error. Thanks.

     
  • Dave Heller

    Dave Heller - 2014-02-28
    • status: open --> pending
     
  • Dave Heller

    Dave Heller - 2014-03-27
    • Release: backlog --> 1.4.8
     
  • Dave Heller

    Dave Heller - 2014-03-27
    • Status: pending --> fixed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks