Month of Kali Off-By-Ones #1 #2 # 3 #4
The following 4 code snippets have classic off-by-one bugs. They don't explicitly nul terminate strings after a strncpy. If strncpy reaches its buffer max, it won't nul terminate. In fact, strncpy's behaviour is quite problematic and prone to this type of bug so OpenBSD introduced strlcpy many years ago and other OSs have done the same.
source package: libaria #1
char myWaitingForDir[2048];
…
AREXPORT void ArClientFileLister::changeToAbsDir(const char *dir)
{
myDataMutex.lock();
strncpy(myWaitingForDir, dir, sizeof(myWaitingForDir));
myLastRequested.setToNow();
//printf("Getting %s\n", myWaitingForDir);
std::string waitingFor = myWaitingForDir;
myDataMutex.unlock();
//myClient->requestOnceWithString("getDirListing", waitingFor.c_str());
getDirListing(waitingFor.c_str());
}
source package: xenomai #2
static inline void xntimer_set_name(xntimer_t *timer, const char *name)
{
#ifdef CONFIG_XENO_OPT_STATS
strncpy(timer->name, name, sizeof(timer->name));
#endif /* CONFIG_XENO_OPT_STATS */
}
source package: service-wrapper #3
char localAddr[128];
...
addr.S_un.S_addr = (u_long)tcpTable->table[i].dwLocalAddr;
strncpy(localAddr, inet_ntoa(addr), sizeof(localAddr));
localPort = ntohs((u_short)tcpTable->table[i].dwLocalPort);
#ifdef GETPORTSTATUS_DEBUG
addr.S_un.S_addr = (u_long)tcpTable->table[i].dwRemoteAddr;
strncpy(remoteAddr, inet_ntoa(addr), sizeof(remoteAddr));
is this a bug? inet_ntoa will be quite small and nul terminate.. I class it as a bad development practice.
source package: nfs-utils #4
static char buf[PATH_MAX];
struct stat st;
char *path;
/* First: test length of name and whether it exists */
if (lstat(parentdir, &st) == -1) {
(void)fprintf(stderr, "%s: Failed to stat %s: %s",
progname, parentdir, strerror(errno));
return false;
}
/* Ensure we have a clean directory pathname */
strncpy(buf, parentdir, sizeof(buf));
path = dirname(buf);
if (*path == '.') {
(void)fprintf(stderr, "%s: Unusable directory %s",
progname, parentdir);
return false;
}
source package: libaria #1
char myWaitingForDir[2048];
…
AREXPORT void ArClientFileLister::changeToAbsDir(const char *dir)
{
myDataMutex.lock();
strncpy(myWaitingForDir, dir, sizeof(myWaitingForDir));
myLastRequested.setToNow();
//printf("Getting %s\n", myWaitingForDir);
std::string waitingFor = myWaitingForDir;
myDataMutex.unlock();
//myClient->requestOnceWithString("getDirListing", waitingFor.c_str());
getDirListing(waitingFor.c_str());
}
source package: xenomai #2
static inline void xntimer_set_name(xntimer_t *timer, const char *name)
{
#ifdef CONFIG_XENO_OPT_STATS
strncpy(timer->name, name, sizeof(timer->name));
#endif /* CONFIG_XENO_OPT_STATS */
}
source package: service-wrapper #3
char localAddr[128];
...
addr.S_un.S_addr = (u_long)tcpTable->table[i].dwLocalAddr;
strncpy(localAddr, inet_ntoa(addr), sizeof(localAddr));
localPort = ntohs((u_short)tcpTable->table[i].dwLocalPort);
#ifdef GETPORTSTATUS_DEBUG
addr.S_un.S_addr = (u_long)tcpTable->table[i].dwRemoteAddr;
strncpy(remoteAddr, inet_ntoa(addr), sizeof(remoteAddr));
is this a bug? inet_ntoa will be quite small and nul terminate.. I class it as a bad development practice.
source package: nfs-utils #4
static char buf[PATH_MAX];
struct stat st;
char *path;
/* First: test length of name and whether it exists */
if (lstat(parentdir, &st) == -1) {
(void)fprintf(stderr, "%s: Failed to stat %s: %s",
progname, parentdir, strerror(errno));
return false;
}
/* Ensure we have a clean directory pathname */
strncpy(buf, parentdir, sizeof(buf));
path = dirname(buf);
if (*path == '.') {
(void)fprintf(stderr, "%s: Unusable directory %s",
progname, parentdir);
return false;
}