From 9a210ca2d50e02bf045866bbb2f44a33a3c48cd9 Mon Sep 17 00:00:00 2001 From: wm4 Date: Tue, 1 Jul 2014 23:10:38 +0200 Subject: [PATCH] Audit and replace all ctype.h uses Something like "char *s = ...; isdigit(s[0]);" triggers undefined behavior, because char can be signed, and thus s[0] can be a negative value. The is*() functions require unsigned char _or_ EOF. EOF is a special value outside of unsigned char range, thus the argument to the is*() functions can't be a char. This undefined behavior can actually trigger crashes if the implementation of these functions e.g. uses lookup tables, which are then indexed with out-of-range values. Replace all uses with our own custom mp_is*() functions added with misc/ctype.h. As a bonus, these functions are locale-independent. (Although currently, we _require_ C locale for other reasons.) --- audio/out/ao_alsa.c | 1 - audio/out/ao_coreaudio_utils.c | 4 ++-- bstr/bstr.c | 8 ++++---- demux/demux_mkv.c | 1 - demux/demux_subreader.c | 1 - demux/mf.c | 4 ++-- input/input.c | 1 - misc/ctype.h | 19 +++++++++++++++++++ options/m_option.c | 1 - options/parse_configfile.c | 16 ++++++++-------- player/configfiles.c | 4 ++-- player/main.c | 1 - player/timeline/tl_cue.c | 1 - player/timeline/tl_mpv_edl.c | 1 - stream/dvb_tune.c | 1 - stream/stream_dvb.c | 4 ++-- stream/stream_dvd.c | 1 - stream/stream_pvr.c | 1 - stream/tv.c | 4 ++-- sub/find_subfiles.c | 6 +++--- sub/sd_microdvd.c | 1 - sub/sd_srt.c | 12 ++++++------ video/out/gl_common.c | 1 - video/out/pnm_loader.c | 6 +++--- video/out/vo_opengl_old.c | 4 ++-- 25 files changed, 55 insertions(+), 49 deletions(-) create mode 100644 misc/ctype.h diff --git a/audio/out/ao_alsa.c b/audio/out/ao_alsa.c index 25a051fee8..cd46e52806 100644 --- a/audio/out/ao_alsa.c +++ b/audio/out/ao_alsa.c @@ -31,7 +31,6 @@ #include #include #include -#include #include #include diff --git a/audio/out/ao_coreaudio_utils.c b/audio/out/ao_coreaudio_utils.c index 11f89422ff..052f63d608 100644 --- a/audio/out/ao_coreaudio_utils.c +++ b/audio/out/ao_coreaudio_utils.c @@ -30,7 +30,7 @@ char *fourcc_repr(void *talloc_ctx, uint32_t code) { // Extract FourCC letters from the uint32_t and finde out if it's a valid // code that is made of letters. - char fcc[4] = { + unsigned char fcc[4] = { (code >> 24) & 0xFF, (code >> 16) & 0xFF, (code >> 8) & 0xFF, @@ -39,7 +39,7 @@ char *fourcc_repr(void *talloc_ctx, uint32_t code) bool valid_fourcc = true; for (int i = 0; i < 4; i++) - if (!isprint(fcc[i])) + if (fcc[i] >= 32 && fcc[i] < 128) valid_fourcc = false; char *repr; diff --git a/bstr/bstr.c b/bstr/bstr.c index 964934a100..de8f285161 100644 --- a/bstr/bstr.c +++ b/bstr/bstr.c @@ -18,7 +18,6 @@ #include #include -#include #include #include #include @@ -28,6 +27,7 @@ #include "talloc.h" #include "common/common.h" +#include "misc/ctype.h" #include "bstr/bstr.h" int bstrcmp(struct bstr str1, struct bstr str2) @@ -104,7 +104,7 @@ int bstr_find(struct bstr haystack, struct bstr needle) struct bstr bstr_lstrip(struct bstr str) { - while (str.len && isspace(*str.start)) { + while (str.len && mp_isspace(*str.start)) { str.start++; str.len--; } @@ -114,7 +114,7 @@ struct bstr bstr_lstrip(struct bstr str) struct bstr bstr_strip(struct bstr str) { str = bstr_lstrip(str); - while (str.len && isspace(str.start[str.len - 1])) + while (str.len && mp_isspace(str.start[str.len - 1])) str.len--; return str; } @@ -242,7 +242,7 @@ bool bstr_eatstart(struct bstr *s, struct bstr prefix) void bstr_lower(struct bstr str) { for (int i = 0; i < str.len; i++) - str.start[i] = tolower(str.start[i]); + str.start[i] = mp_tolower(str.start[i]); } int bstr_sscanf(struct bstr str, const char *format, ...) diff --git a/demux/demux_mkv.c b/demux/demux_mkv.c index e3991372f9..25591cd60a 100644 --- a/demux/demux_mkv.c +++ b/demux/demux_mkv.c @@ -23,7 +23,6 @@ #include #include -#include #include #include #include diff --git a/demux/demux_subreader.c b/demux/demux_subreader.c index d3a792c9d2..6eb58f0033 100644 --- a/demux/demux_subreader.c +++ b/demux/demux_subreader.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/demux/mf.c b/demux/mf.c index 5edc98043a..d687c3cb19 100644 --- a/demux/mf.c +++ b/demux/mf.c @@ -16,7 +16,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include #include #include #include @@ -30,6 +29,7 @@ #include "talloc.h" #include "common/msg.h" #include "stream/stream.h" +#include "misc/ctype.h" #include "options/path.h" #include "mf.h" @@ -55,7 +55,7 @@ mf_t *open_mf_pattern(void *talloc_ctx, struct mp_log *log, char *filename) while (fgets(fname, 512, lst_f)) { /* remove spaces from end of fname */ char *t = fname + strlen(fname) - 1; - while (t > fname && isspace((unsigned char)*t)) + while (t > fname && mp_isspace(*t)) *(t--) = 0; if (!mp_path_exists(fname)) { mp_verbose(log, "file not found: '%s'\n", fname); diff --git a/input/input.c b/input/input.c index 212a6ff92d..983cb84fe3 100644 --- a/input/input.c +++ b/input/input.c @@ -28,7 +28,6 @@ #include #include #include -#include #include #include diff --git a/misc/ctype.h b/misc/ctype.h new file mode 100644 index 0000000000..cbff799285 --- /dev/null +++ b/misc/ctype.h @@ -0,0 +1,19 @@ +#ifndef MP_CTYPE_H_ +#define MP_CTYPE_H_ + +// Roughly follows C semantics, but doesn't account for EOF, allows char as +// parameter, and is locale independent (always uses "C" locale). + +static inline int mp_isprint(char c) { return (unsigned char)c >= 32; } +static inline int mp_isspace(char c) { return c == ' ' || c == '\f' || c == '\n' || + c == '\r' || c == '\t' || c =='\v'; } +static inline int mp_isupper(char c) { return c >= 'A' && c <= 'Z'; } +static inline int mp_islower(char c) { return c >= 'a' && c <= 'z'; } +static inline int mp_isdigit(char c) { return c >= '0' && c <= '9'; } +static inline int mp_isalpha(char c) { return mp_isupper(c) || mp_islower(c); } +static inline int mp_isalnum(char c) { return mp_isalpha(c) || mp_isdigit(c); } + +static inline char mp_tolower(char c) { return mp_isupper(c) ? c - 'A' + 'a' : c; } +static inline char mp_toupper(char c) { return mp_islower(c) ? c - 'a' + 'A' : c; } + +#endif diff --git a/options/m_option.c b/options/m_option.c index 3baaaa6762..52dd18b9d2 100644 --- a/options/m_option.c +++ b/options/m_option.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include diff --git a/options/parse_configfile.c b/options/parse_configfile.c index 1e12a5c47f..112a26b115 100644 --- a/options/parse_configfile.c +++ b/options/parse_configfile.c @@ -22,13 +22,13 @@ #include #include #include -#include #include #include "osdep/io.h" #include "parse_configfile.h" #include "common/msg.h" +#include "misc/ctype.h" #include "m_option.h" #include "m_config.h" @@ -95,7 +95,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, line_pos = 0; /* skip whitespaces */ - while (isspace(line[line_pos])) + while (mp_isspace(line[line_pos])) ++line_pos; /* EOL / comment */ @@ -103,7 +103,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, continue; /* read option. */ - for (opt_pos = 0; isprint(line[line_pos]) && + for (opt_pos = 0; mp_isprint(line[line_pos]) && line[line_pos] != ' ' && line[line_pos] != '#' && line[line_pos] != '='; /* NOTHING */) { @@ -133,7 +133,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, } /* skip whitespaces */ - while (isspace(line[line_pos])) + while (mp_isspace(line[line_pos])) ++line_pos; param_pos = 0; @@ -145,7 +145,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, param_set = true; /* whitespaces... */ - while (isspace(line[line_pos])) + while (mp_isspace(line[line_pos])) ++line_pos; /* read the parameter */ @@ -187,8 +187,8 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, } } - for (param_pos = 0; isprint(line[line_pos]) - && !isspace(line[line_pos]) + for (param_pos = 0; mp_isprint(line[line_pos]) + && !mp_isspace(line[line_pos]) && line[line_pos] != '#'; /* NOTHING */) { param[param_pos++] = line[line_pos++]; if (param_pos >= MAX_PARAM_LEN) { @@ -202,7 +202,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile, param_done: - while (isspace(line[line_pos])) + while (mp_isspace(line[line_pos])) ++line_pos; } param[param_pos] = '\0'; diff --git a/player/configfiles.c b/player/configfiles.c index dab26b9df2..d1c79c9c9d 100644 --- a/player/configfiles.c +++ b/player/configfiles.c @@ -22,7 +22,6 @@ #include #include #include -#include #include @@ -34,6 +33,7 @@ #include "common/global.h" #include "common/encode.h" #include "common/msg.h" +#include "misc/ctype.h" #include "options/path.h" #include "options/m_config.h" #include "options/parse_configfile.h" @@ -267,7 +267,7 @@ static bool needs_config_quoting(const char *s) { for (int i = 0; s && s[i]; i++) { unsigned char c = s[i]; - if (!isprint(c) || isspace(c) || c == '#' || c == '\'' || c == '"') + if (!mp_isprint(c) || mp_isspace(c) || c == '#' || c == '\'' || c == '"') return true; } return false; diff --git a/player/main.c b/player/main.c index 0a18df1a4e..aebfa06fb1 100644 --- a/player/main.c +++ b/player/main.c @@ -21,7 +21,6 @@ #include #include #include -#include #include #include diff --git a/player/timeline/tl_cue.c b/player/timeline/tl_cue.c index 6731ab4058..d5e8b08164 100644 --- a/player/timeline/tl_cue.c +++ b/player/timeline/tl_cue.c @@ -20,7 +20,6 @@ #include #include #include -#include #include "talloc.h" diff --git a/player/timeline/tl_mpv_edl.c b/player/timeline/tl_mpv_edl.c index aba9738d53..78fc8b7cdc 100644 --- a/player/timeline/tl_mpv_edl.c +++ b/player/timeline/tl_mpv_edl.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include "talloc.h" diff --git a/stream/dvb_tune.c b/stream/dvb_tune.c index 7065a77aa3..056592bceb 100644 --- a/stream/dvb_tune.c +++ b/stream/dvb_tune.c @@ -25,7 +25,6 @@ #include #include -#include #include #include #include diff --git a/stream/stream_dvb.c b/stream/stream_dvb.c index d6ebddb944..87b2495a51 100644 --- a/stream/stream_dvb.c +++ b/stream/stream_dvb.c @@ -34,7 +34,6 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA #include #include #include -#include #include #include #include @@ -45,6 +44,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA #include #include "osdep/io.h" +#include "misc/ctype.h" #include "stream.h" #include "options/m_config.h" @@ -177,7 +177,7 @@ static dvb_channels_list *dvb_get_channels(struct mp_log *log, char *filename, i { fields = sscanf(&line[k], sat_conf, &ptr->freq, &ptr->pol, &ptr->diseqc, &ptr->srate, vpid_str, apid_str); - ptr->pol = toupper(ptr->pol); + ptr->pol = mp_toupper(ptr->pol); ptr->freq *= 1000UL; ptr->srate *= 1000UL; ptr->tone = -1; diff --git a/stream/stream_dvd.c b/stream/stream_dvd.c index 2195189c42..9773e14fa1 100644 --- a/stream/stream_dvd.c +++ b/stream/stream_dvd.c @@ -18,7 +18,6 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include #include #include #include diff --git a/stream/stream_pvr.c b/stream/stream_pvr.c index db355131b1..217a27849a 100644 --- a/stream/stream_pvr.c +++ b/stream/stream_pvr.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include diff --git a/stream/tv.c b/stream/tv.c index 17b1f9ecb9..726b7d0880 100644 --- a/stream/tv.c +++ b/stream/tv.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -38,6 +37,7 @@ #include "common/msg.h" +#include "misc/ctype.h" #include "options/m_option.h" #include "options/m_config.h" @@ -628,7 +628,7 @@ static int open_tv(tvi_handle_t *tvh) int channel = 0; if (tvh->tv_param->channel) { - if (isdigit(*tvh->tv_param->channel)) + if (mp_isdigit(*tvh->tv_param->channel)) /* if tvh->tv_param->channel begins with a digit interpret it as a number */ channel = atoi(tvh->tv_param->channel); else diff --git a/sub/find_subfiles.c b/sub/find_subfiles.c index 9313bf379d..ade267cfa6 100644 --- a/sub/find_subfiles.c +++ b/sub/find_subfiles.c @@ -1,16 +1,16 @@ #include #include #include -#include #include #include "osdep/io.h" +#include "common/common.h" #include "common/global.h" #include "common/msg.h" +#include "misc/ctype.h" #include "options/options.h" #include "options/path.h" -#include "common/common.h" #include "sub/find_subfiles.h" static const char *const sub_exts[] = {"utf", "utf8", "utf-8", "idx", "sub", "srt", @@ -75,7 +75,7 @@ static struct bstr guess_lang_from_filename(struct bstr name) if (name.start[i] == ')' || name.start[i] == ']') i--; - while (i >= 0 && isalpha(name.start[i])) { + while (i >= 0 && mp_isalpha(name.start[i])) { n++; if (n > 3) return (struct bstr){NULL, 0}; diff --git a/sub/sd_microdvd.c b/sub/sd_microdvd.c index 6e6a9c31a8..5de9a1814b 100644 --- a/sub/sd_microdvd.c +++ b/sub/sd_microdvd.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include "common/msg.h" diff --git a/sub/sd_srt.c b/sub/sd_srt.c index 733a27d0f4..0ca807f312 100644 --- a/sub/sd_srt.c +++ b/sub/sd_srt.c @@ -24,11 +24,11 @@ #include #include #include -#include -#include +#include "common/common.h" #include "common/msg.h" #include "bstr/bstr.h" +#include "misc/ctype.h" #include "sd.h" struct line { @@ -259,7 +259,7 @@ static int read_attr(char **s, struct bstr *attr, struct bstr *val) attr->start = *s; attr->len = eq - *s; for (int i = 0; i < attr->len; i++) - if (!isalnum(attr->start[i])) + if (!mp_isalnum(attr->start[i])) return -1; val->start = eq + 1; bool quoted = val->start[0] == '"'; @@ -290,7 +290,7 @@ static void convert_subrip(struct sd *sd, const char *orig, while (*line && new_line.len < new_line.bufsize - 1) { char *orig_line = line; - for (int i = 0; i < FF_ARRAY_ELEMS(subrip_basic_tags); i++) { + for (int i = 0; i < MP_ARRAY_SIZE(subrip_basic_tags); i++) { const struct tag_conv *tag = &subrip_basic_tags[i]; int from_len = strlen(tag->from); if (strncmp(line, tag->from, from_len) == 0) { @@ -331,7 +331,7 @@ static void convert_subrip(struct sd *sd, const char *orig, } } } else if (strncmp(line, " #include #include -#include #include #include #include diff --git a/video/out/pnm_loader.c b/video/out/pnm_loader.c index 048461e51f..70afe0fa23 100644 --- a/video/out/pnm_loader.c +++ b/video/out/pnm_loader.c @@ -33,7 +33,7 @@ #include #include #include -#include +#include "misc/ctype.h" #include "pnm_loader.h" /** @@ -48,7 +48,7 @@ static void ppm_skip(FILE *f) { comment = 1; if (c == '\n') comment = 0; - } while (c != EOF && (isspace(c) || comment)); + } while (c != EOF && (mp_isspace(c) || comment)); if (c != EOF) ungetc(c, f); } @@ -77,7 +77,7 @@ uint8_t *read_pnm(FILE *f, int *width, int *height, if (fscanf(f, "%u", &m) != 1) return NULL; val = fgetc(f); - if (!isspace(val)) + if (!mp_isspace(val)) return NULL; if (w > MAXDIM || h > MAXDIM) return NULL; diff --git a/video/out/vo_opengl_old.c b/video/out/vo_opengl_old.c index d9c7e8baed..87ba48068a 100644 --- a/video/out/vo_opengl_old.c +++ b/video/out/vo_opengl_old.c @@ -28,12 +28,12 @@ #include #include #include -#include #include #include "config.h" #include "talloc.h" #include "common/msg.h" +#include "misc/ctype.h" #include "options/m_option.h" #include "vo.h" #include "video/vfcap.h" @@ -538,7 +538,7 @@ static void replace_var_str(char **text, const char *name, const char *replace) nextvar += namelen; // try not to replace prefixes of other vars (e.g. $foo vs. $foo_bar) char term = nextvar[0]; - if (isalnum(term) || term == '_') + if (mp_isalnum(term) || term == '_') continue; int prelength = until - *text; int postlength = nextvar - *text;