#120 Buffer Overflow Exploitation

[Anonymous]

It was found that the C based File Upload application (ICAP) did not adhere to secure best practice coding standards with the most severe issue being rated as critical, and concerning the use of functions that could lead to buffer overflow exploitation.

PFA the list of component which causes the issue.

Can you please put in a patch for these issues or release it as a new version of code that rectifies the deficient coding practices highlighted within this report.

[chtsanti]

We can discuss if you like the reported possible issues, however in all of these cases I found no real buffer overflows. In all reported cases there are checks and code which prevents the buffer overflow.
Are you used a tool to make this list?
Which tool?

[Anonymous]

hi @chtsanti , the test was done by an external vendor we are reaching out to them.
Will let you know the details of the tool as soon as we get the details.

[chtsanti]

Thank you.

Various tools have found in many cases serious problems in c-icap in the past. But in many cases produces false alarms. If you are worry about c-icap code, I checked the list you sent and I did not see any real problem, so I believe they are just false alarms.

In the past I made fixes in the code just to avoid such false alarms and if it is easy, I will do it again for the list you sent me. Exactly because such tools are important it is better to keep them happy at least to avoid noise while you are (I am) checking.

I must note that there are a number of (possible) issues which are fixed in git repository and waiting to be merged to the 0.5.x branch.
A new c-icap-0.5.6 release will be released soon.

[Anonymous]

Hi @chtsanti,Thanks for your response. Following is the response that we got from the external vendor:-

Issues were identified through manual review of code and no tool was used.
Project need to ensure that the following insecure functions are not utilised "strcpy", "sprintf", "strcat", "gets" or "getpw" within the ICAP codebase (or any C based codebase). Microsoft has these functions listed on their banned function list and does not recommend their use in any C program. More secure alternatives to these functions exist and these were listed within the sheet attached earlier

[chtsanti]

Using the gets and getpw is a bug.

But just using the other functions you are listed (strcpy, sprintf, strcat) does not mean that you are making a bug. Their use is prohibited because if they are not used with care can cause bugs.
In all cases they are (still) used in c-icap there are the required checks for lenghts.

The following cases are not bugs:
char buf[1024]; sprintf(buf, "Hello");
char *d = malloc(strlen(s) + 1); if (d) strcpy(d, s);

[chtsanti]

Their use is prohibited because..

Their use is discouraged because...

[chtsanti]

In the master branch all the calls to sprintf, strcpy, strcat, ctime_r and asctime_r are replaced with the safer similar alternatives. You can check commits ee45def5, 7d36aa81, 5d2835ca and 8e87a694.
These commits probably will not apeared to a c-icap-0.5.x release but will be included in the c-icap-0.6.x releases.

During the checks for possible bugs at least three cases found where it is possible to cause buffer overflows. These are fixed with the c-icap master commits 673b61bb, b5ac639c and 211458fb.
These are will be included in the next c-icap-0.5.x relase.

Probably similar work required for the c-icap-modules too.

Thank you to turned my attention to these problems.