Hello. At Wed, 22 Apr 2020 19:48:07 -0300, Ranier Vilela <ranier...@gmail.com> wrote in > Hi, > strncpy, it is not a safe function and has the risk of corrupting memory. > On ecpg lib, two sources, make use of strncpy risk, this patch tries to fix. > > 1. Make room for the last null-characte; > 2. Copies Maximum number of characters - 1. > > per Coverity.
- strncpy(sqlca->sqlstate, sqlstate, sizeof(sqlca->sqlstate)); + sqlca->sqlstate[sizeof(sqlca->sqlstate) - 1] = '\0'; + strncpy(sqlca->sqlstate, sqlstate, sizeof(sqlca->sqlstate) - 1); Did you look at the definition and usages of the struct member? sqlstate is a char[5], which is to be filled with 5-letter SQLSTATE code not terminated by NUL, which can be shorter if NUL is found anywhere (I'm not sure there's actually a case of a shorter state code). If you put NUL to the 5th element of the array, you break the content. The existing code looks perfect to me. - strncpy(sqlca->sqlerrm.sqlerrmc, message, sizeof(sqlca->sqlerrm.sqlerrmc)); - sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] = 0; + sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1] = '\0'; + strncpy(sqlca->sqlerrm.sqlerrmc, message, sizeof(sqlca->sqlerrm.sqlerrmc) - 1); The existing strncpy then terminating by NUL works fine. I don't think there's any point in doing the reverse way. Actually sizeof(sqlca->sqlerrm.sqlerrmc) - 1 is enough for the length but the existing code is not necessarily a bug. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
