important issues found by covscan
Dos2Unix / Unix2Dos - Text file format converters
Brought to you by:
waterlan
Error: SECURE_TEMP (CWE-377): dos2unix-7.4.0/common.c:992: secure_temp: Calling "mkstemp" without securely setting umask first. # 990| goto make_failed; # 991| #else # 992|-> if ((fd = mkstemp(fname_str)) == -1) # 993| goto make_failed; # 994| Error: RESOURCE_LEAK (CWE-772): dos2unix-7.4.0/common.c:959: alloc_fn: Storage is returned from allocation function "strdup". dos2unix-7.4.0/common.c:959: var_assign: Assigning: "cpy" = storage returned from "strdup(OutFN)". dos2unix-7.4.0/common.c:975: identity_transfer: Passing "cpy" as argument 1 to function "dirname", which returns that argument. dos2unix-7.4.0/common.c:975: noescape: Resource "cpy" is not freed or pointed-to in "dirname". dos2unix-7.4.0/common.c:975: var_assign: Assigning: "dir" = storage returned from "dirname(cpy)". dos2unix-7.4.0/common.c:977: noescape: Resource "dir" is not freed or pointed-to in "strlen". dos2unix-7.4.0/common.c:1004: leaked_storage: Variable "dir" going out of scope leaks the storage it points to. dos2unix-7.4.0/common.c:1004: leaked_storage: Variable "cpy" going out of scope leaks the storage it points to. # 1002| free(*fname_ret); # 1003| *fname_ret = NULL; # 1004|-> return NULL; # 1005| } # 1006| Error: CHECKED_RETURN (CWE-252): dos2unix-7.4.0/common.c:1979: check_return: Calling "fputc" without checking return value (as is done elsewhere 20 out of 22 times). dos2unix-7.4.0/common.c:2675: example_checked: Example 1: "fputc(c_lead, f)" has its value checked in "fputc(c_lead, f) == -1". dos2unix-7.4.0/common.c:2750: example_checked: Example 2: "fputc(0, f)" has its value checked in "fputc(0, f) == -1". dos2unix-7.4.0/common.c:2792: example_checked: Example 3: "fputc(mbs[i], f)" has its value checked in "fputc(mbs[i], f) == -1". dos2unix-7.4.0/dos2unix.c:147: example_checked: Example 4: "fputc(CurChar, ipOutF)" has its value checked in "fputc(CurChar, ipOutF) == -1". dos2unix-7.4.0/dos2unix.c:154: example_checked: Example 5: "fputc(10, ipOutF)" has its value checked in "fputc(10, ipOutF) == -1". # 1977| } # 1978| if (ipFlag->file_info & INFO_PRINT0) # 1979|-> fputc(0, stdout); # 1980| else # 1981| D2U_UTF8_FPRINTF(stdout, "\n"); Error: CHECKED_RETURN (CWE-252): dos2unix-7.4.0/common.c:2010: check_return: Calling "fputc" without checking return value (as is done elsewhere 20 out of 22 times). dos2unix-7.4.0/common.c:2675: example_checked: Example 1: "fputc(c_lead, f)" has its value checked in "fputc(c_lead, f) == -1". dos2unix-7.4.0/common.c:2750: example_checked: Example 2: "fputc(0, f)" has its value checked in "fputc(0, f) == -1". dos2unix-7.4.0/common.c:2792: example_checked: Example 3: "fputc(mbs[i], f)" has its value checked in "fputc(mbs[i], f) == -1". dos2unix-7.4.0/dos2unix.c:147: example_checked: Example 4: "fputc(CurChar, ipOutF)" has its value checked in "fputc(CurChar, ipOutF) == -1". dos2unix-7.4.0/dos2unix.c:154: example_checked: Example 5: "fputc(10, ipOutF)" has its value checked in "fputc(10, ipOutF) == -1". # 2008| } # 2009| if (ipFlag->file_info & INFO_PRINT0) # 2010|-> fputc(0, stdout); # 2011| else # 2012| D2U_UTF8_FPRINTF(stdout, "\n"); Error: CLANG_WARNING: dos2unix-7.4.0/common.c:2542:11: warning: Value stored to 'conversion_error' is never read # conversion_error = GetFileInfo(argv[ArgIdx], pFlag, progname); # ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ dos2unix-7.4.0/common.c:2542:11: note: Value stored to 'conversion_error' is never read # conversion_error = GetFileInfo(argv[ArgIdx], pFlag, progname); # ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # 2540| } else { # 2541| if (pFlag->file_info) { # 2542|-> conversion_error = GetFileInfo(argv[ArgIdx], pFlag, progname); # 2543| print_messages_info(pFlag, argv[ArgIdx], progname); # 2544| } else { Error: TAINTED_SCALAR (CWE-20): dos2unix-7.4.0/dos2unix.c:383: tainted_data_return: Function "fgetc" returns tainted data. dos2unix-7.4.0/dos2unix.c:383: var_assign: Assigning: "TempChar" = "fgetc", which taints "TempChar". dos2unix-7.4.0/dos2unix.c:384: lower_bounds: Checking lower bounds of signed scalar "TempChar" by "TempChar < 32". dos2unix-7.4.0/dos2unix.c:402: tainted_data: Using tainted variable "TempChar" as an index to pointer "ConvTable". # 400| if (TempChar == '\x0a') /* Count all DOS and Unix line breaks */ # 401| ++line_nr; # 402|-> if (fputc(ConvTable[TempChar], ipOutF) == EOF) { # 403| RetVal = -1; # 404| d2u_putc_error(ipFlag,progname); Error: TAINTED_SCALAR (CWE-20): dos2unix-7.4.0/dos2unix.c:420: tainted_data_return: Function "fgetc" returns tainted data. dos2unix-7.4.0/dos2unix.c:420: var_assign: Assigning: "TempChar" = "fgetc", which taints "TempChar". dos2unix-7.4.0/dos2unix.c:421: lower_bounds: Checking lower bounds of signed scalar "TempChar" by "TempChar < 32". dos2unix-7.4.0/dos2unix.c:439: tainted_data: Using tainted variable "TempChar" as an index to pointer "ConvTable". # 437| if (TempChar == '\x0a') /* Count all DOS and Unix line breaks */ # 438| ++line_nr; # 439|-> if(fputc(ConvTable[TempChar], ipOutF) == EOF) { # 440| RetVal = -1; # 441| d2u_putc_error(ipFlag,progname); Error: TAINTED_SCALAR (CWE-20): dos2unix-7.4.0/unix2dos.c:337: tainted_data_return: Function "fgetc" returns tainted data. dos2unix-7.4.0/unix2dos.c:337: var_assign: Assigning: "TempChar" = "fgetc", which taints "TempChar". dos2unix-7.4.0/unix2dos.c:338: lower_bounds: Checking lower bounds of signed scalar "TempChar" by "TempChar < 32". dos2unix-7.4.0/unix2dos.c:379: tainted_data: Using tainted variable "TempChar" as an index to pointer "ConvTable". # 377| if (TempChar == '\x0a') /* Count all DOS and Unix line breaks */ # 378| ++line_nr; # 379|-> if (fputc(ConvTable[TempChar], ipOutF) == EOF) { /* put LF or other char */ # 380| RetVal = -1; # 381| d2u_putc_error(ipFlag,progname); Error: TAINTED_SCALAR (CWE-20): dos2unix-7.4.0/unix2dos.c:397: tainted_data_return: Function "fgetc" returns tainted data. dos2unix-7.4.0/unix2dos.c:397: var_assign: Assigning: "TempChar" = "fgetc", which taints "TempChar". dos2unix-7.4.0/unix2dos.c:398: lower_bounds: Checking lower bounds of signed scalar "TempChar" by "TempChar < 32". dos2unix-7.4.0/unix2dos.c:414: tainted_data: Using tainted variable "TempChar" as an index to pointer "ConvTable". # 412| } # 413| if (TempChar != '\x0a') { /* Not an LF */ # 414|-> if(fputc(ConvTable[TempChar], ipOutF) == EOF) { # 415| RetVal = -1; # 416| d2u_putc_error(ipFlag,progname);
Anonymous
the following patch should fix the RESOURCE_LEAK and CHECKED_RETURN errors.
I'm not sure to fix the Error: TAINTED_SCALA. I know we need to check the TempChar before calling fputc.
Hi Than,
Thanks for reporting.
I will have a look soon.
best regards,
Erwin
The range of TempChar is correct (0..255), because it is an unsigned char cast to an int.
Adding a check is not needed and will slow dos2unix down.
I propose to waive that error.
Fix uploaded. See [48217a]
Related
Commit: [48217a]