mirror of
https://github.com/mpv-player/mpv
synced 2025-02-16 12:17:12 +00:00
demux_mf: improve format string processing
Before this commit, the user could specify a printf format string
which wasn't verified, and could result in:
- Undefined behavior due to missing or non-matching arguments.
- Buffer overflow due to untested result length.
The offending code was added at commit 103a9609
(2002, mplayer svn):
git-svn-id: svn://svn.mplayerhq.hu/mplayer/trunk@4566 b3059339-0415-0410-9bf9-f77b7e298cf2
It moved around but was not modified meaningfully until now.
Now we reject all conversion specifiers at the format except %%
and a simple subset of the valid specifiers. Also, we now use
snprintf to avoid buffer overflow.
The format string is provided by the user as part of mf:// URI.
Report and initial patch by Stefan Schiller.
Patch reviewed by @jeeb, @sfan5, Stefan Schiller.
This commit is contained in:
parent
0728b51498
commit
cb3fa04bcb
@ -121,7 +121,8 @@ static mf_t *open_mf_pattern(void *talloc_ctx, struct demuxer *d, char *filename
|
|||||||
goto exit_mf;
|
goto exit_mf;
|
||||||
}
|
}
|
||||||
|
|
||||||
char *fname = talloc_size(mf, strlen(filename) + 32);
|
size_t fname_avail = strlen(filename) + 32;
|
||||||
|
char *fname = talloc_size(mf, fname_avail);
|
||||||
|
|
||||||
#if HAVE_GLOB
|
#if HAVE_GLOB
|
||||||
if (!strchr(filename, '%')) {
|
if (!strchr(filename, '%')) {
|
||||||
@ -148,10 +149,44 @@ static mf_t *open_mf_pattern(void *talloc_ctx, struct demuxer *d, char *filename
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
// We're using arbitrary user input as printf format with 1 int argument.
|
||||||
|
// Any format which uses exactly 1 int argument would be valid, but for
|
||||||
|
// simplicity we reject all conversion specifiers except %% and simple
|
||||||
|
// integer specifier: %[.][NUM]d where NUM is 1-3 digits (%.d is valid)
|
||||||
|
const char *f = filename;
|
||||||
|
int MAXDIGS = 3, nspec = 0, bad_spec = 0, c;
|
||||||
|
|
||||||
|
while (nspec < 2 && (c = *f++)) {
|
||||||
|
if (c != '%')
|
||||||
|
continue;
|
||||||
|
if (*f != '%') {
|
||||||
|
nspec++; // conversion specifier which isn't %%
|
||||||
|
if (*f == '.')
|
||||||
|
f++;
|
||||||
|
for (int ndig = 0; mp_isdigit(*f) && ndig < MAXDIGS; ndig++, f++)
|
||||||
|
/* no-op */;
|
||||||
|
if (*f != 'd') {
|
||||||
|
bad_spec++; // not int, or beyond our validation capacity
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// *f is '%' or 'd'
|
||||||
|
f++;
|
||||||
|
}
|
||||||
|
|
||||||
|
// nspec==0 (zero specifiers) is rejected because fname wouldn't advance.
|
||||||
|
if (bad_spec || nspec != 1) {
|
||||||
|
mp_err(log, "unsupported expr format: '%s'\n", filename);
|
||||||
|
goto exit_mf;
|
||||||
|
}
|
||||||
|
|
||||||
mp_info(log, "search expr: %s\n", filename);
|
mp_info(log, "search expr: %s\n", filename);
|
||||||
|
|
||||||
while (error_count < 5) {
|
while (error_count < 5) {
|
||||||
sprintf(fname, filename, count++);
|
if (snprintf(fname, fname_avail, filename, count++) >= fname_avail) {
|
||||||
|
mp_err(log, "format result too long: '%s'\n", filename);
|
||||||
|
goto exit_mf;
|
||||||
|
}
|
||||||
if (!mp_path_exists(fname)) {
|
if (!mp_path_exists(fname)) {
|
||||||
error_count++;
|
error_count++;
|
||||||
mp_verbose(log, "file not found: '%s'\n", fname);
|
mp_verbose(log, "file not found: '%s'\n", fname);
|
||||||
|
Loading…
Reference in New Issue
Block a user