Skip to content

replace potentially unsafe C library calls flagged by OpenBSD linker #2981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rdmark wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

replace potentially unsafe C library calls flagged by OpenBSD linker #2981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4556850
replace unsafe C library calls flagged by OpenBSD linker
rdmark May 10, 2026
f5b0bf1
afpd: use full appl buffer size AFPOBJ_TMPSIZ+1 in strlcpy
rdmark May 10, 2026
c79954e
afpd: use messages MAXPATHLEN+1 stack buffer for message filename
rdmark May 10, 2026
ff853b5
afpd: guard AppleDesktop strlcat truncation in setdeskmode and setdes…
rdmark May 10, 2026
4f3f62a
testsuite: fix afp helper length/data mismatch in cnamewrap
rdmark May 10, 2026
1d407d3
afpd: extract AppleDesktop desk_foreach to eliminate duplicated trave…
rdmark May 10, 2026
8c44539
replace unsafe functions with bounded equivalents in afpd and testsuite
rdmark May 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/nad/ftw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ static char *mystpcpy(char *dest, const char *src, size_t dest_size)

return dest;
} else {
strcpy(dest, src);
strlcpy(dest, src, dest_size);
return dest + src_len;
}
}
Expand Down
16 changes: 14 additions & 2 deletions etc/afpd/appl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,13 @@ int afp_addappl(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf _U_,
}

dtf = dtfile(vol, creator, ".appl.temp");

if (!dtf) {
return AFPERR_PARAM;
}

tempfile = obj->oldtmp;
strcpy(tempfile, dtf);
strlcpy(tempfile, dtf, AFPOBJ_TMPSIZ + 1);
Comment thread
rdmark marked this conversation as resolved.

if ((tfd = open(tempfile, O_RDWR | O_CREAT, 0666)) < 0) {
return AFPERR_PARAM;
Expand Down Expand Up @@ -400,6 +405,13 @@ int afp_rmvappl(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf _U_,
}

dtf = dtfile(vol, creator, ".appl.temp");

if (!dtf) {
close(sa.sdt_fd);
sa.sdt_fd = -1;
return AFPERR_PARAM;
}

/* Use existing tempfile buffers but check if the path fits */
tempfile = obj->oldtmp;

Expand All @@ -409,7 +421,7 @@ int afp_rmvappl(AFPObj *obj, char *ibuf, size_t ibuflen _U_, char *rbuf _U_,
return AFPERR_PARAM;
}

strcpy(tempfile, dtf);
strlcpy(tempfile, dtf, AFPOBJ_TMPSIZ + 1);

if ((tfd = open(tempfile, O_RDWR | O_CREAT, 0666)) < 0) {
close(sa.sdt_fd);
Expand Down
239 changes: 121 additions & 118 deletions etc/afpd/desktop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,44 +53,34 @@

#define EXEC_MODE (S_IXGRP | S_IXUSR | S_IXOTH)

int setdeskmode(const struct vol *vol, const mode_t mode)
{
EC_INIT;
char wd[MAXPATHLEN + 1];
struct stat st;
char modbuf[12 + 1], *m;
struct dirent *deskp, *subp;
DIR *desk, *sub;

if (!dir_rx_set(mode)) {
/* want to remove read and search access to owner it will screw the volume */
return -1 ;
}

if (getcwd(wd, MAXPATHLEN) == NULL) {
return -1;
}

bstring dtpath = bfromcstr(vol->v_dbpath);
bcatcstr(dtpath, "/" APPLEDESKTOP);
EC_NEG1(chdir(cfrombstr(dtpath)));

if ((desk = opendir(".")) == NULL) {
if (chdir(wd) < 0) {
LOG(log_error, logtype_afpd, "setdeskmode: chdir %s: %s", wd, strerror(errno));
}
/* ----------------------
* Callback type for desk_foreach. Called once per AppleDesktop entry
* (inner creator-code files and outer hex dirs alike).
*/
typedef void (*desk_entry_fn)(const char *path, void *ctx);

EC_FAIL;
}
/*
* Iterate the two-level AppleDesktop hex-dir tree rooted at the current
* working directory, calling fn(path, ctx) for every inner entry and then
* for each outer hex dir. Skips entries whose name would overflow the
* modbuf[13] path buffer sized for the format (≤2-char hex dir + "/" +
* ≤8-char creator code).
*/
static void desk_foreach(DIR *desk, desk_entry_fn fn, void *ctx)
{
char modbuf[12 + 1], *m;
const struct dirent *deskp, *subp;
DIR *sub;

for (deskp = readdir(desk); deskp != NULL; deskp = readdir(desk)) {
if (strcmp(deskp->d_name, ".") == 0 ||
strcmp(deskp->d_name, "..") == 0 || strlen(deskp->d_name) > 2) {
strcmp(deskp->d_name, "..") == 0 ||
strlen(deskp->d_name) > 2) {
continue;
}

strcpy(modbuf, deskp->d_name);
strcat(modbuf, "/");
strlcpy(modbuf, deskp->d_name, sizeof(modbuf));
strlcat(modbuf, "/", sizeof(modbuf));
m = strchr(modbuf, '\0');

if ((sub = opendir(deskp->d_name)) == NULL) {
Expand All @@ -104,47 +94,94 @@ int setdeskmode(const struct vol *vol, const mode_t mode)
}

*m = '\0';
strcat(modbuf, subp->d_name);

/* XXX: need to preserve special modes */
if (lstat(modbuf, &st) < 0) {
LOG(log_error, logtype_afpd, "setdeskmode: stat %s: %s", fullpathname(modbuf),
strerror(errno));
if (strlcat(modbuf, subp->d_name, sizeof(modbuf)) >= sizeof(modbuf)) {
LOG(log_warning, logtype_afpd,
"desk_foreach: skipping entry with unexpected name length: %s",
subp->d_name);
continue;
}

if (S_ISDIR(st.st_mode)) {
if (ochmod(modbuf,
DIRBITS | mode,
&st,
vol_syml_opt(vol) | vol_chmod_opt(vol)
) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskmode: chmod %s: %s", fullpathname(modbuf),
strerror(errno));
}
} else if (ochmod(modbuf,
mode & ~EXEC_MODE,
&st,
vol_syml_opt(vol) | vol_chmod_opt(vol)
) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskmode: chmod %s: %s", fullpathname(modbuf),
strerror(errno));
}
fn(modbuf, ctx);
}

closedir(sub);
fn(deskp->d_name, ctx);
}
}

struct deskmode_ctx {
const struct vol *vol;
mode_t mode;
};

static void deskmode_entry(const char *path, void *vctx)
{
const struct deskmode_ctx *ctx = vctx;
struct stat st;

/* XXX: need to preserve special modes */
if (lstat(path, &st) < 0) {
LOG(log_error, logtype_afpd, "setdeskmode: stat %s: %s",
fullpathname(path), strerror(errno));
return;
}

mode_t m = S_ISDIR(st.st_mode) ? (DIRBITS | ctx->mode) :
(ctx->mode & ~EXEC_MODE);

/* XXX: need to preserve special modes */
if (ochmod(deskp->d_name,
DIRBITS | mode,
NULL,
vol_syml_opt(vol) | vol_chmod_opt(vol)
) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskmode: chmod %s: %s",
fullpathname(deskp->d_name), strerror(errno));
if (ochmod((char *)path, m, &st,
vol_syml_opt(ctx->vol) | vol_chmod_opt(ctx->vol)) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskmode: chmod %s: %s",
fullpathname(path), strerror(errno));
}
}

struct deskowner_ctx {
uid_t uid;
gid_t gid;
};

static void deskowner_entry(const char *path, void *vctx)
{
const struct deskowner_ctx *ctx = vctx;

/* XXX: add special any uid, ignore group bits */
if (chown(path, ctx->uid, ctx->gid) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskowner: chown %s: %s",
fullpathname(path), strerror(errno));
}
}

int setdeskmode(const struct vol *vol, const mode_t mode)
{
EC_INIT;
char wd[MAXPATHLEN + 1];
DIR *desk;
struct deskmode_ctx ctx = { .vol = vol, .mode = mode };

if (!dir_rx_set(mode)) {
/* want to remove read and search access to owner it will screw the volume */
return -1;
}

if (getcwd(wd, MAXPATHLEN) == NULL) {
return -1;
}

bstring dtpath = bfromcstr(vol->v_dbpath);
bcatcstr(dtpath, "/" APPLEDESKTOP);
EC_NEG1(chdir(cfrombstr(dtpath)));

if ((desk = opendir(".")) == NULL) {
if (chdir(wd) < 0) {
LOG(log_error, logtype_afpd, "setdeskmode: chdir %s: %s", wd, strerror(errno));
}

EC_FAIL;
}

desk_foreach(desk, deskmode_entry, &ctx);
closedir(desk);

if (chdir(wd) < 0) {
Expand All @@ -170,10 +207,9 @@ int setdeskmode(const struct vol *vol, const mode_t mode)
int setdeskowner(const struct vol *vol, uid_t uid, gid_t gid)
{
EC_INIT;
char wd[MAXPATHLEN + 1];
char modbuf[12 + 1], *m;
struct dirent *deskp, *subp;
DIR *desk, *sub;
char wd[MAXPATHLEN + 1];
DIR *desk;
struct deskowner_ctx ctx = { .uid = uid, .gid = gid };

if (getcwd(wd, MAXPATHLEN) == NULL) {
return -1;
Expand All @@ -191,46 +227,7 @@ int setdeskowner(const struct vol *vol, uid_t uid, gid_t gid)
EC_FAIL;
}

for (deskp = readdir(desk); deskp != NULL; deskp = readdir(desk)) {
if (strcmp(deskp->d_name, ".") == 0 ||
strcmp(deskp->d_name, "..") == 0 ||
strlen(deskp->d_name) > 2) {
continue;
}

strcpy(modbuf, deskp->d_name);
strcat(modbuf, "/");
m = strchr(modbuf, '\0');

if ((sub = opendir(deskp->d_name)) == NULL) {
continue;
}

for (subp = readdir(sub); subp != NULL; subp = readdir(sub)) {
if (strcmp(subp->d_name, ".") == 0 ||
strcmp(subp->d_name, "..") == 0) {
continue;
}

*m = '\0';
strcat(modbuf, subp->d_name);

/* XXX: add special any uid, ignore group bits */
if (chown(modbuf, uid, gid) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskown: chown %s: %s", fullpathname(modbuf),
strerror(errno));
}
}

closedir(sub);

/* XXX: add special any uid, ignore group bits */
if (chown(deskp->d_name, uid, gid) < 0 && errno != EPERM) {
LOG(log_error, logtype_afpd, "setdeskowner: chown %s: %s",
deskp->d_name, strerror(errno));
}
}

desk_foreach(desk, deskowner_entry, &ctx);
closedir(desk);

if (chdir(wd) < 0) {
Expand Down Expand Up @@ -808,7 +805,6 @@ static const char hexdig[] = "0123456789abcdef";
char *dtfile(const struct vol *vol, uint8_t creator[], char *ext)
{
static char path[MAXPATHLEN + 1];
char *p;
unsigned int i;

if (vol == NULL || vol->v_dbpath == NULL) {
Expand All @@ -822,29 +818,36 @@ char *dtfile(const struct vol *vol, uint8_t creator[], char *ext)
return NULL;
}

for (p = path; *p != '\0'; p++)
;
/* Build "X/CCCC" suffix (2-char hex dir + '/' + up to 8-char creator + NUL) */
char suffix[12];
char *q = suffix;

if (!isprint(creator[0]) || creator[0] == '/') {
*p++ = hexdig[(creator[0] & 0xf0) >> 4];
*p++ = hexdig[creator[0] & 0x0f];
*q++ = hexdig[(creator[0] & 0xf0) >> 4];
*q++ = hexdig[creator[0] & 0x0f];
} else {
*p++ = creator[0];
*q++ = creator[0];
}

*p++ = '/';
*q++ = '/';

for (i = 0; i < sizeof(CreatorType); i++) {
if (!isprint(creator[i]) || creator[i] == '/') {
*p++ = hexdig[(creator[i] & 0xf0) >> 4];
*p++ = hexdig[creator[i] & 0x0f];
*q++ = hexdig[(creator[i] & 0xf0) >> 4];
*q++ = hexdig[creator[i] & 0x0f];
} else {
*p++ = creator[i];
*q++ = creator[i];
}
}

*p = '\0';
strcat(path, ext);
*q = '\0';

if (strlcat(path, suffix, sizeof(path)) >= sizeof(path) ||
strlcat(path, ext, sizeof(path)) >= sizeof(path)) {
LOG(log_error, logtype_afpd, "dtfile: path too long");
return NULL;
}

return path;
}

Expand All @@ -863,7 +866,7 @@ char *mtoupath(const struct vol *vol, char *mpath, cnid_t did, int utf8)
uint16_t flags;

if (*mpath == '\0') {
strcpy(upath, ".");
strlcpy(upath, ".", sizeof(upath));
return upath;
}

Expand Down
4 changes: 2 additions & 2 deletions etc/afpd/directory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ static int cname_mtouname(const struct vol *vol, struct dir *dir,
return -1;
}

strcpy(ret->m_name, temp);
strlcpy(ret->m_name, temp, MAXPATHLEN + 1);
}
}

Expand All @@ -404,7 +404,7 @@ static int cname_mtouname(const struct vol *vol, struct dir *dir,
*/
if ((t = utompath(vol, ret->u_name, fileid, utf8_encoding(vol->v_obj)))) {
/* at last got our view of mac name */
strcpy(ret->m_name, t);
strlcpy(ret->m_name, t, MAXPATHLEN + 1);
}
}
} /* afp_version >= 30 */
Expand Down
Loading