Skip to content

M
mpd
Commit 263a9d58 authored by Eric Wong's avatar Eric Wong
Browse files
Options

remove clumsy strncpy use

strncpy isn't really safe because it doesn't guarantee null termination, and we have had to work around it in several places. strlcpy (from OpenBSD) isn't great, either because it often leaves errors going unchecked (by truncating strings). So we'll add the pathcpy_trunc() function with is basically strlcpy with a hardcoded MAXPATHLEN as the limit, and we'll acknowledge truncation since we only work on paths and MAXPATHLEN should be set correctly by the system headers[1]. file-specific notes: inputStream_http: eyeballing the changes here, it seems to look alright but I haven't actually tested it myself. ls: don't even bother printing a file if the filename is too long (and when is it ever?) since we won't be able to read it anyways. metadataChunk: it's only metadata, and it's only for showin the user, so truncating it here souldn't be a big issue. memset to zero in init is unecessary, so lets not waste cycles [1] - If the system headers are screwed up, then we're majorly screwed regardless of what we do :x git-svn-id: https://svn.musicpd.org/mpd/trunk@4491 09075e82-0dd4-0310-85a5-a0d7c8717e4f
parent 4144afe5
Show whitespace changes
Inline Side-by-side
Showing with 76 additions and 59 deletions
src/decode.c
View file @ 263a9d58
...@@ -112,9 +112,7 @@ static int calculateCrossFadeChunks(PlayerControl * pc, AudioFormat * af) ...@@ -112,9 +112,7 @@ static int calculateCrossFadeChunks(PlayerControl * pc, AudioFormat * af)
{ \ { \
decodeWaitedOn = 0; \ decodeWaitedOn = 0; \
if(openAudioDevice(&(cb->audioFormat))<0) { \ if(openAudioDevice(&(cb->audioFormat))<0) { \
strncpy(pc->erroredUrl, pc->utf8url, \ pathcpy_trunc(pc->erroredUrl, pc->utf8url); \
MAXPATHLEN); \
pc->erroredUrl[MAXPATHLEN] = '\0'; \
pc->error = PLAYER_ERROR_AUDIO; \ pc->error = PLAYER_ERROR_AUDIO; \
ERROR("problems opening audio device while playing \"%s\"\n", pc->utf8url); \ ERROR("problems opening audio device while playing \"%s\"\n", pc->utf8url); \
quitDecode(pc,dc); \ quitDecode(pc,dc); \
...@@ -129,8 +127,7 @@ static int calculateCrossFadeChunks(PlayerControl * pc, AudioFormat * af) ...@@ -129,8 +127,7 @@ static int calculateCrossFadeChunks(PlayerControl * pc, AudioFormat * af)
cb->audioFormat.sampleRate; \ cb->audioFormat.sampleRate; \
} \ } \
else if(dc->state!=DECODE_STATE_START || decode_pid <= 0) { \ else if(dc->state!=DECODE_STATE_START || decode_pid <= 0) { \
strncpy(pc->erroredUrl, pc->utf8url, MAXPATHLEN); \ pathcpy_trunc(pc->erroredUrl, pc->utf8url); \
pc->erroredUrl[MAXPATHLEN] = '\0'; \
pc->error = PLAYER_ERROR_FILE; \ pc->error = PLAYER_ERROR_FILE; \
quitDecode(pc,dc); \ quitDecode(pc,dc); \
return; \ return; \
...@@ -145,15 +142,13 @@ static int waitOnDecode(PlayerControl * pc, DecoderControl * dc, ...@@ -145,15 +142,13 @@ static int waitOnDecode(PlayerControl * pc, DecoderControl * dc,
OutputBuffer * cb, int *decodeWaitedOn) OutputBuffer * cb, int *decodeWaitedOn)
{ {
MpdTag *tag = NULL; MpdTag *tag = NULL;
strncpy(pc->currentUrl, pc->utf8url, MAXPATHLEN); pathcpy_trunc(pc->currentUrl, pc->utf8url);
pc->currentUrl[MAXPATHLEN] = '\0';
while (decode_pid > 0 && dc->start) while (decode_pid > 0 && dc->start)
my_usleep(10000); my_usleep(10000);
if (dc->start || dc->error != DECODE_ERROR_NOERROR) { if (dc->start || dc->error != DECODE_ERROR_NOERROR) {
strncpy(pc->erroredUrl, pc->utf8url, MAXPATHLEN); pathcpy_trunc(pc->erroredUrl, pc->utf8url);
pc->erroredUrl[MAXPATHLEN] = '\0';
pc->error = PLAYER_ERROR_FILE; pc->error = PLAYER_ERROR_FILE;
quitDecode(pc, dc); quitDecode(pc, dc);
return -1; return -1;
...@@ -229,9 +224,7 @@ static int decodeSeek(PlayerControl * pc, DecoderControl * dc, ...@@ -229,9 +224,7 @@ static int decodeSeek(PlayerControl * pc, DecoderControl * dc,
if(pause) pc->state = PLAYER_STATE_PAUSE; \ if(pause) pc->state = PLAYER_STATE_PAUSE; \
else { \ else { \
if(openAudioDevice(NULL)<0) { \ if(openAudioDevice(NULL)<0) { \
strncpy(pc->erroredUrl, pc->utf8url, \ pathcpy_trunc(pc->erroredUrl, pc->utf8url); \
MAXPATHLEN); \
pc->erroredUrl[MAXPATHLEN] = '\0'; \
pc->error = PLAYER_ERROR_AUDIO; \ pc->error = PLAYER_ERROR_AUDIO; \
ERROR("problems opening audio device while playing \"%s\"\n", pc->utf8url); \ ERROR("problems opening audio device while playing \"%s\"\n", pc->utf8url); \
quitDecode(pc,dc); \ quitDecode(pc,dc); \
...@@ -286,8 +279,7 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb, ...@@ -286,8 +279,7 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb,
copyMpdTagToOutputBuffer(cb, NULL); copyMpdTagToOutputBuffer(cb, NULL);
strncpy(dc->utf8url, pc->utf8url, MAXPATHLEN); pathcpy_trunc(dc->utf8url, pc->utf8url);
dc->utf8url[MAXPATHLEN] = '\0';
if (openInputStream(&inStream, path) < 0) { if (openInputStream(&inStream, path) < 0) {
dc->error = DECODE_ERROR_FILE; dc->error = DECODE_ERROR_FILE;
...@@ -396,8 +388,7 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb, ...@@ -396,8 +388,7 @@ static void decodeStart(PlayerControl * pc, OutputBuffer * cb,
} }
if (ret < 0 || ret == DECODE_ERROR_UNKTYPE) { if (ret < 0 || ret == DECODE_ERROR_UNKTYPE) {
strncpy(pc->erroredUrl, dc->utf8url, MAXPATHLEN); pathcpy_trunc(pc->erroredUrl, dc->utf8url);
pc->erroredUrl[MAXPATHLEN] = '\0';
if (ret != DECODE_ERROR_UNKTYPE) if (ret != DECODE_ERROR_UNKTYPE)
dc->error = DECODE_ERROR_FILE; dc->error = DECODE_ERROR_FILE;
else { else {
...@@ -439,8 +430,7 @@ static int decoderInit(PlayerControl * pc, OutputBuffer * cb, ...@@ -439,8 +430,7 @@ static int decoderInit(PlayerControl * pc, OutputBuffer * cb,
/* END OF CHILD */ /* END OF CHILD */
} else if (decode_pid < 0) { } else if (decode_pid < 0) {
unblockSignals(); unblockSignals();
strncpy(pc->erroredUrl, pc->utf8url, MAXPATHLEN); pathcpy_trunc(pc->erroredUrl, pc->utf8url);
pc->erroredUrl[MAXPATHLEN] = '\0';
pc->error = PLAYER_ERROR_SYSTEM; pc->error = PLAYER_ERROR_SYSTEM;
return -1; return -1;
} }
......
src/inputStream_http.c
View file @ 263a9d58
...@@ -306,15 +306,15 @@ static int parseUrl(InputStreamHTTPData * data, char *url) ...@@ -306,15 +306,15 @@ static int parseUrl(InputStreamHTTPData * data, char *url)
if (colon && colon < at) { if (colon && colon < at) {
user = malloc(colon - temp + 1); user = malloc(colon - temp + 1);
strncpy(user, temp, colon - temp); memcpy(user, temp, colon - temp);
user[colon - temp] = '\0'; user[colon - temp] = '\0';
passwd = malloc(at - colon); passwd = malloc(at - colon);
strncpy(passwd, colon + 1, at - colon - 1); memcpy(passwd, colon + 1, at - colon - 1);
passwd[at - colon - 1] = '\0'; passwd[at - colon - 1] = '\0';
} else { } else {
user = malloc(at - temp + 1); user = malloc(at - temp + 1);
strncpy(user, temp, at - temp); memcpy(user, temp, at - temp);
user[at - temp] = '\0'; user[at - temp] = '\0';
passwd = strdup(""); passwd = strdup("");
...@@ -346,7 +346,7 @@ static int parseUrl(InputStreamHTTPData * data, char *url) ...@@ -346,7 +346,7 @@ static int parseUrl(InputStreamHTTPData * data, char *url)
return -1; return -1;
data->host = malloc(len); data->host = malloc(len);
strncpy(data->host, temp, len - 1); memcpy(data->host, temp, len - 1);
data->host[len - 1] = '\0'; data->host[len - 1] = '\0';
/* fetch the port */ /* fetch the port */
if (colon && (!slash || slash != colon + 1)) { if (colon && (!slash || slash != colon + 1)) {
...@@ -354,7 +354,7 @@ static int parseUrl(InputStreamHTTPData * data, char *url) ...@@ -354,7 +354,7 @@ static int parseUrl(InputStreamHTTPData * data, char *url)
if (slash) if (slash)
len -= strlen(slash); len -= strlen(slash);
data->port = malloc(len + 1); data->port = malloc(len + 1);
strncpy(data->port, colon + 1, len); memcpy(data->port, colon + 1, len);
data->port[len] = '\0'; data->port[len] = '\0';
DEBUG(__FILE__ ": Port: %s\n", data->port); DEBUG(__FILE__ ": Port: %s\n", data->port);
} else { } else {
......
src/ls.c
View file @ 263a9d58
...@@ -128,12 +128,14 @@ int lsPlaylists(int fd, char *utf8path) ...@@ -128,12 +128,14 @@ int lsPlaylists(int fd, char *utf8path)
strcat(s, "/"); strcat(s, "/");
while ((ent = readdir(dir))) { while ((ent = readdir(dir))) {
size_t len = strlen(ent->d_name) + 1;
dup = ent->d_name; dup = ent->d_name;
if (dup[0] != '.' && if (mpd_likely(len <= maxlen) &&
dup[0] != '.' &&
(suff = strlen(dup) - suflen) > 0 && (suff = strlen(dup) - suflen) > 0 &&
dup[suff] == '.' && dup[suff] == '.' &&
strcmp(dup + suff + 1, PLAYLIST_FILE_SUFFIX) == 0) { strcmp(dup + suff + 1, PLAYLIST_FILE_SUFFIX) == 0) {
strncpy(s + actlen, ent->d_name, maxlen); memcpy(s + actlen, ent->d_name, len);
if (stat(s, &st) == 0) { if (stat(s, &st) == 0) {
if (S_ISREG(st.st_mode)) { if (S_ISREG(st.st_mode)) {
if (list == NULL) if (list == NULL)
......
src/metadataChunk.c
View file @ 263a9d58
...@@ -17,13 +17,12 @@ ...@@ -17,13 +17,12 @@
*/ */
#include "metadataChunk.h" #include "metadataChunk.h"
#include "gcc.h"
#include <string.h> #include <string.h>
static void initMetadataChunk(MetadataChunk * chunk) static void initMetadataChunk(MetadataChunk * chunk)
{ {
memset(chunk, 0, sizeof(MetadataChunk));
chunk->name = -1; chunk->name = -1;
chunk->artist = -1; chunk->artist = -1;
chunk->album = -1; chunk->album = -1;
...@@ -54,8 +53,12 @@ MpdTag *metadataChunkToMpdTagDup(MetadataChunk * chunk) ...@@ -54,8 +53,12 @@ MpdTag *metadataChunkToMpdTagDup(MetadataChunk * chunk)
if(element < 0 && string && (slen = strlen(string)) && \ if(element < 0 && string && (slen = strlen(string)) && \
pos < METADATA_BUFFER_LENGTH-1) \ pos < METADATA_BUFFER_LENGTH-1) \
{ \ { \
strncpy(chunk->buffer+pos, string, \ size_t len = slen; \
METADATA_BUFFER_LENGTH-1-pos); \ size_t max = METADATA_BUFFER_LENGTH - 1 - pos; \
if (mpd_unlikely(len > max)) \
len = max; \
memcpy(chunk->buffer+pos, string, len); \
*(chunk->buffer+pos+len) = '\0'; \
element = pos; \ element = pos; \
pos += slen+1; \ pos += slen+1; \
} \ } \
......
src/path.c
View file @ 263a9d58
...@@ -37,10 +37,9 @@ ...@@ -37,10 +37,9 @@
#endif #endif
#endif #endif
char *musicDir; const char *musicDir;
char *playlistDir; static const char *playlistDir;
static char *fsCharset = NULL;
char *fsCharset = NULL;
static char *pathConvCharset(char *to, char *from, char *str) static char *pathConvCharset(char *to, char *from, char *str)
{ {
...@@ -205,39 +204,59 @@ void finishPaths(void) ...@@ -205,39 +204,59 @@ void finishPaths(void)
fsCharset = NULL; fsCharset = NULL;
} }
char *rmp2amp(char *relativePath) static char *pfx_path(const char *path, const char *pfx, const size_t pfx_len)
{ {
static char absolutePath[MAXPATHLEN + 1]; static char ret[MAXPATHLEN+1];
size_t rp_len = strlen(path);
memset(absolutePath, 0, MAXPATHLEN + 1); /* check for the likely condition first: */
if (mpd_likely((pfx_len + rp_len) < MAXPATHLEN)) {
memcpy(ret, pfx, pfx_len);
memcpy(ret + pfx_len, path, rp_len + 1);
return ret;
}
strncpy(absolutePath, musicDir, MAXPATHLEN); /* unlikely, return an empty string because truncating would
strncat(absolutePath, relativePath, MAXPATHLEN - strlen(musicDir)); * also be wrong... break early and break loudly (the system
* headers are likely screwed, not mpd) */
ERROR("Cannot prefix '%s' to '%s', max: %d", pfx, path, MAXPATHLEN);
ret[0] = '\0';
return ret;
}
return absolutePath; char *rmp2amp(char *relativePath)
{
size_t pfx_len = strlen(musicDir);
return pfx_path(relativePath, musicDir, pfx_len);
} }
char *rpp2app(char *relativePath) char *rpp2app(char *relativePath)
{ {
static char absolutePath[MAXPATHLEN + 1]; size_t pfx_len = strlen(playlistDir);
return pfx_path(relativePath, playlistDir, pfx_len);
memset(absolutePath, 0, MAXPATHLEN + 1); }
strncpy(absolutePath, playlistDir, MAXPATHLEN); /* this is actually like strlcpy (OpenBSD), but we don't actually want to
strncat(absolutePath, relativePath, MAXPATHLEN - strlen(musicDir)); * blindly use it everywhere, only for paths that are OK to truncate (for
* error reporting and such */
void pathcpy_trunc(char *dest, const char *src)
{
size_t len = strlen(src);
return absolutePath; if (mpd_unlikely(len > MAXPATHLEN))
len = MAXPATHLEN;
memcpy(dest, src, len);
dest[len] = '\0';
} }
char *parentPath(char *path) char *parentPath(char *path)
{ {
static char parentPath[MAXPATHLEN + 1]; static char parentPath[MAXPATHLEN+1];
char *c; char *c;
memset(parentPath, 0, MAXPATHLEN + 1); pathcpy_trunc(parentPath, path);
strncpy(parentPath, path, MAXPATHLEN); c = strrchr(parentPath,'/');
c = strrchr(parentPath, '/');
if (c == NULL) if (c == NULL)
parentPath[0] = '\0'; parentPath[0] = '\0';
else { else {
......
src/path.h
View file @ 263a9d58
...@@ -23,7 +23,7 @@ ...@@ -23,7 +23,7 @@
#include <sys/param.h> #include <sys/param.h>
extern char *musicDir; extern const char *musicDir;
void initPaths(); void initPaths();
...@@ -51,4 +51,10 @@ char *parentPath(char *path); ...@@ -51,4 +51,10 @@ char *parentPath(char *path);
/* strips extra "///" and leading "/" and trailing "/" */ /* strips extra "///" and leading "/" and trailing "/" */
char *sanitizePathDup(char *path); char *sanitizePathDup(char *path);
/* this is actually like strlcpy (OpenBSD), but we don't actually want to
* blindly use it everywhere, only for paths that are OK to truncate (for
* error reporting and such.
* dest must be MAXPATHLEN+1 bytes large (which is standard in mpd) */
void pathcpy_trunc(char *dest, const char *src);
#endif #endif
src/player.c
View file @ 263a9d58
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
*/ */
#include "player.h" #include "player.h"
#include "path.h"
#include "decode.h" #include "decode.h"
#include "command.h" #include "command.h"
#include "interface.h" #include "interface.h"
...@@ -170,8 +171,7 @@ int playerPlay(int fd, Song * song) ...@@ -170,8 +171,7 @@ int playerPlay(int fd, Song * song)
copyMpdTagToMetadataChunk(song->tag, &(pc->fileMetadataChunk)); copyMpdTagToMetadataChunk(song->tag, &(pc->fileMetadataChunk));
strncpy(pc->utf8url, getSongUrl(song), MAXPATHLEN); pathcpy_trunc(pc->utf8url, getSongUrl(song));
pc->utf8url[MAXPATHLEN] = '\0';
pc->play = 1; pc->play = 1;
if (getPlayerPid() == 0 && playerInit() < 0) { if (getPlayerPid() == 0 && playerInit() < 0) {
...@@ -337,8 +337,7 @@ int queueSong(Song * song) ...@@ -337,8 +337,7 @@ int queueSong(Song * song)
PlayerControl *pc = &(getPlayerData()->playerControl); PlayerControl *pc = &(getPlayerData()->playerControl);
if (pc->queueState == PLAYER_QUEUE_BLANK) { if (pc->queueState == PLAYER_QUEUE_BLANK) {
strncpy(pc->utf8url, getSongUrl(song), MAXPATHLEN); pathcpy_trunc(pc->utf8url, getSongUrl(song));
pc->utf8url[MAXPATHLEN] = '\0';
if (song->tag) if (song->tag)
pc->fileTime = song->tag->time; pc->fileTime = song->tag->time;
...@@ -408,8 +407,7 @@ int playerSeek(int fd, Song * song, float time) ...@@ -408,8 +407,7 @@ int playerSeek(int fd, Song * song, float time)
copyMpdTagToMetadataChunk(song->tag, &(pc->fileMetadataChunk)); copyMpdTagToMetadataChunk(song->tag, &(pc->fileMetadataChunk));
strncpy(pc->utf8url, getSongUrl(song), MAXPATHLEN); pathcpy_trunc(pc->utf8url, getSongUrl(song));
pc->utf8url[MAXPATHLEN] = '\0';
} }
if (pc->error == PLAYER_ERROR_NOERROR) { if (pc->error == PLAYER_ERROR_NOERROR) {
......
src/tag.c
View file @ 263a9d58
...@@ -612,10 +612,9 @@ int mpdTagsAreEqual(MpdTag * tag1, MpdTag * tag2) ...@@ -612,10 +612,9 @@ int mpdTagsAreEqual(MpdTag * tag1, MpdTag * tag2)
static void appendToTagItems(MpdTag * tag, int type, char *value, int len) static void appendToTagItems(MpdTag * tag, int type, char *value, int len)
{ {
int i = tag->numOfItems; int i = tag->numOfItems;
char *dup = malloc(len + 1);
char *dup; memcpy(dup, value, len);
dup = malloc(len + 1);
strncpy(dup, value, len);
dup[len] = '\0'; dup[len] = '\0';
fixUtf8(dup); fixUtf8(dup);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment