| Message ID | 20230723180303.8000-3-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 509ce40f188734ec74078ebdd8d71f80116d9eaf |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/vmixdec: Check for end of input in decode_dcac() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure_x86 | warning | Failed to apply patch |
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
Would this patch be an opportunity to set `cpl->content_title_utf8` to an empty string at fill_content_title() at libavformat/imf_cpl.c if xmlNodeListGetString() returns NULL? It could be done as a separate patch alternatively. LGTM otherwise. On Sun, Jul 23, 2023 at 11:03 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > Fixes: NULL pointer dereference > Fixes: 60166/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5998301577871360 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/imf_cpl.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c > index fe975c2f0c..69155d786d 100644 > --- a/libavformat/imf_cpl.c > +++ b/libavformat/imf_cpl.c > @@ -75,6 +75,8 @@ int ff_imf_xml_read_uuid(xmlNodePtr element, AVUUID uuid) > int ret = 0; > > xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); > + if (!element_text) > + return AVERROR_INVALIDDATA; > ret = av_uuid_urn_parse(element_text, uuid); > if (ret) > ret = AVERROR_INVALIDDATA; > @@ -88,7 +90,7 @@ int ff_imf_xml_read_rational(xmlNodePtr element, AVRational *rational) > int ret = 0; > > xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); > - if (sscanf(element_text, "%i %i", &rational->num, &rational->den) != 2) > + if (element_text == NULL || sscanf(element_text, "%i %i", &rational->num, &rational->den) != 2) > ret = AVERROR_INVALIDDATA; > xmlFree(element_text); > > @@ -100,7 +102,7 @@ int ff_imf_xml_read_uint32(xmlNodePtr element, uint32_t *number) > int ret = 0; > > xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); > - if (sscanf(element_text, "%" PRIu32, number) != 1) > + if (element_text == NULL || sscanf(element_text, "%" PRIu32, number) != 1) > ret = AVERROR_INVALIDDATA; > xmlFree(element_text); > > @@ -245,6 +247,8 @@ static int fill_timecode(xmlNodePtr cpl_element, FFIMFCPL *cpl) > return AVERROR_INVALIDDATA; > > tc_str = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); > + if (!tc_str) > + return AVERROR_INVALIDDATA; > ret = parse_cpl_tc_type(tc_str, comps); > xmlFree(tc_str); > if (ret) > -- > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Sun, Jul 23, 2023 at 12:55:46PM -0700, Pierre-Anthony Lemieux wrote: > Would this patch be an opportunity to set `cpl->content_title_utf8` to > an empty string at fill_content_title() at libavformat/imf_cpl.c if > xmlNodeListGetString() returns NULL? It could be done as a separate > patch alternatively. ill send a seperate patch for this > > LGTM otherwise. will apply thx [...]
diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c index fe975c2f0c..69155d786d 100644 --- a/libavformat/imf_cpl.c +++ b/libavformat/imf_cpl.c @@ -75,6 +75,8 @@ int ff_imf_xml_read_uuid(xmlNodePtr element, AVUUID uuid) int ret = 0; xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); + if (!element_text) + return AVERROR_INVALIDDATA; ret = av_uuid_urn_parse(element_text, uuid); if (ret) ret = AVERROR_INVALIDDATA; @@ -88,7 +90,7 @@ int ff_imf_xml_read_rational(xmlNodePtr element, AVRational *rational) int ret = 0; xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); - if (sscanf(element_text, "%i %i", &rational->num, &rational->den) != 2) + if (element_text == NULL || sscanf(element_text, "%i %i", &rational->num, &rational->den) != 2) ret = AVERROR_INVALIDDATA; xmlFree(element_text); @@ -100,7 +102,7 @@ int ff_imf_xml_read_uint32(xmlNodePtr element, uint32_t *number) int ret = 0; xmlChar *element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); - if (sscanf(element_text, "%" PRIu32, number) != 1) + if (element_text == NULL || sscanf(element_text, "%" PRIu32, number) != 1) ret = AVERROR_INVALIDDATA; xmlFree(element_text); @@ -245,6 +247,8 @@ static int fill_timecode(xmlNodePtr cpl_element, FFIMFCPL *cpl) return AVERROR_INVALIDDATA; tc_str = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); + if (!tc_str) + return AVERROR_INVALIDDATA; ret = parse_cpl_tc_type(tc_str, comps); xmlFree(tc_str); if (ret)
Fixes: NULL pointer dereference Fixes: 60166/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5998301577871360 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/imf_cpl.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)