174189 - flawfinder warnings in internationalization

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Stephen P. Morse]

Heikki ran flawfinder (http://www.dwheeler.com/flawfinder) on Mozilla 1.0.1 
branch.

flawfinder found 5 warnings in internationalization code (4497-4500, 4502). Go 
through that list and for each warning:

* If it is false positive, comment here why it is not an issue
* If it is a real issue, make patch for it here and let's get them checked in

In addition to checking the branch, also check the trunk.

4497) intl/locale/src/mac/nsDateTimeFormatMac.cpp:435 [4] (buffer) strcat: does 
not check for buffer overflows. Consider using strncat or strlcat.

4498) intl/locale/src/nsLocaleService.cpp:406 [4] (buffer) strcpy: does not 
check for buffer overflows. Consider using strncpy or strlcpy.

4499) intl/locale/src/nsLocaleService.cpp:462 [4] (buffer) strcpy: does not 
check for buffer overflows. Consider using strncpy or strlcpy.

4500) intl/locale/src/nsLocaleService.cpp:471 [4] (buffer) strcpy: does not 
check for buffer overflows. Consider using strncpy or strlcpy. 

4502) intl/uconv/public/nsICharsetConverterManager.h:121 [4] (buffer) sprintf: 
does not check for buffer overflows. Use snprintf or vsnprintf.

Comment 1

[Rui Xu]

code issue, QA to [e-mail] for now.

Comment 2

[Stephen P. Morse]

Here's an analysis of the five warnings reported here:

4497) intl/locale/src/mac/nsDateTimeFormatMac.cpp:435 [4] (buffer) strcat:

not in 1.0.1 branch nor the trunk
is in 1.0.0 branch
it looks like flawfinder was not run on the 1.0.1 branch after all
so this is not a problem on trunk or on 1.0.1 branch
might have been a problem in 1.0.0 branch


--------------------

4498) intl/locale/src/nsLocaleService.cpp:406 [4] (buffer) strcpy:
406: strcpy(input, acceptLanguage);

input is dimensioned to be [strlen(acceptLanguage)+1]
therefore no problem

--------------------

4499) intl/locale/src/nsLocaleService.cpp:462 [4] (buffer) strcpy:
462: strcpy(acceptLanguageList[i],ptrLanguage[i]);

acceptLanguageList[i] is dimensioned to be [NSILOCALE_MAX_ACCEPT_LENGTH]
  which is 18
the only value ever assigned to acceptLanguageList[i] is cPtr
cPtr is initialized with nsCRT::strtok(input,",",&cPtr2)
since input is initialized to acceptLanguage, and acceptLanguage is passed
  in as a parameter, this could be a problem
will attach patch for it

-------------------

4500) intl/locale/src/nsLocaleService.cpp:471 [4] (buffer) strcpy:
471: strcpy(acceptLanguageList[countLang++],cPtr);

same as 4499 above

--------------------

4502) intl/uconv/public/nsICharsetConverterManager.h:121 [4] (buffer) sprintf:
not in 1.0.1 branch nor the trunk
is in 1.0.0 branch
it looks like flawfinder was not run on the 1.0.1 branch after all
so this is not a problem on trunk or on 1.0.1 branch

in 1.0.0 branch:  sprintf(buff, "%s/%s", "software/netscape/intl/uconv", cid_string)
buff is dimensioned to 1024
first %s is a fixed string of 28 characters
second %s is a cid
so the combination of the two could never exceed 1024 so this was safe

Comment 3

[Stephen P. Morse]

Attachment: fix flawfinder warning 4499 and 4500

Comment 4

[harishd]

Comment on attachment 104349 [details] [diff] [review]
fix flawfinder warning 4499 and 4500

r=harishd

Comment 5

[Daniel Veditz [:dveditz]]

Comment on attachment 104349 [details] [diff] [review]
fix flawfinder warning 4499 and 4500

The second change is unnecessary, protected by the if() above. But harmless and
will shut Flawfinder up about it.

sr=dveditz

Comment 6

[Stephen P. Morse]

Checked in.