On Tue, Aug 19, 2025 at 06:33:03PM +0300, [email protected] wrote: > The branch, master has been updated > via 4c3f94f2651ce4d6834fbef10e5c287f25720ac9 (commit) > via fe496b0308f1911138698b92bbafac582fa455d7 (commit) > via 535a07d14e16eb2930d2b1e16605eafa95959139 (commit) > via 8132ee046d1a03c5913759c50fce7beda8d3d627 (commit) > via 5d4f873ff31f86f9ae1927f37ca0fcb78da70512 (commit) > via 1e816ebefee0ef479fa76bd2a1fb9ec5d5e807df (commit) > via 93a8091015978c44462626409af71789080bbef4 (commit) > via 303f60684f25ce63951480ef0f759acf41aac938 (commit) > via 5caaadee7954f2014d9e0a17d1edfb961893bcf1 (commit) > via d3190a64c366a79091fe47fddf93c09a7d988802 (commit) > via 44af3829796427b89c4b76b56652cc5932ada616 (commit) <------------- > we are here > via f5ad1c910c168d05cb01315773ab0bd094c9372f (commit) > via e3aa1154aab2656c91ce61915f79516d9b563b61 (commit) > via c6cc2115f45ac26ae42442f8796996eb410f4028 (commit) > via 52dba25661305e3c4a6209d46aea43cd327c960e (commit) > via bfb17d26306592c85cf0c4e909099c621177b062 (commit) > via ba2ea285e0f270a0a885b414cafaace6a89b9a91 (commit) > via ad77345a5d14862f4701e5ad422b03b14934a5b9 (commit) > via bb90b262d6d23f1bca3587a48abc15b951cbbf05 (commit) > via a99fff4e2d4058c57599ba0b968862af82bdad5b (commit) > from a6b5a382dd7ecdd27c5d0ebba688e1db409d18fd (commit) > [...]
> @@ -540,6 +543,98 @@ static char *iso88591_to_utf8(const char *in, size_t
> size_in)
> return out;
> }
>
> +static int decode_text_to_exif(PNGDecContext *s, const char *txt_utf8)
> +{
> + size_t len = strlen(txt_utf8);
> + const char *ptr = txt_utf8;
> + const char *end = txt_utf8 + len;
> + size_t exif_len = 0;
> + uint8_t *exif_ptr;
> + const uint8_t *exif_end;
> +
> + // first we find a newline
> + while (*ptr++ != '\n') {
> + if (ptr >= end)
> + return AVERROR_BUFFER_TOO_SMALL;
> + }
> +
> + // we check for "exif" and skip over it
> + if (end - ptr < 4 || strncmp("exif", ptr, 4))
> + return AVERROR_INVALIDDATA;
> + ptr += 3;
> +
> + // then we find the next printable non-space character
> + while (!av_isgraph(*++ptr)) {
> + if (ptr >= end)
> + return AVERROR_BUFFER_TOO_SMALL;
> + }
> +
> + // parse the length
> + while (av_isdigit(*ptr)) {
> + size_t nlen = exif_len * 10 + (*ptr - '0');
> + if (nlen < exif_len) // overflow
> + return AVERROR_INVALIDDATA;
> + exif_len = nlen;
> + if (++ptr >= end)
> + return AVERROR_BUFFER_TOO_SMALL;
> + }
> +
> + // then we find the next printable non-space character
> + while (!av_isgraph(*ptr)) {
> + if (++ptr >= end)
> + return AVERROR_BUFFER_TOO_SMALL;
> + }
IIUC the format is
newline, profile name, decimal length, newline, then hex payload.
that can be checked for directly and cleanly
> +
> + // first condition checks for overflow in 2 * exif_len
> + if ((exif_len & ~SIZE_MAX) || end - ptr < 2 * exif_len)
found by and fixed in https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22596
> + return AVERROR_INVALIDDATA;
> + if (exif_len < 10)
> + return AVERROR_INVALIDDATA;
> +
> + av_buffer_unref(&s->exif_data);
> + // the buffer starts with "Exif " which we skip over
> + // we don't use AV_EXIF_EXIF00 because that disagrees
> + // with the eXIf chunk format
> + s->exif_data = av_buffer_alloc(exif_len - 6);
> + if (!s->exif_data)
> + return AVERROR(ENOMEM);
> +
> + // we subtract one because we call ++ptr later
> + // compiler will optimize out the call
> + ptr += strlen("Exif ") * 2 - 1;
that looks just fragile
> +
> + exif_ptr = s->exif_data->data;
> + exif_end = exif_ptr + s->exif_data->size;
> +
> + while (exif_ptr < exif_end) {
> + while (++ptr < end) {
> + if (*ptr >= '0' && *ptr <= '9') {
> + *exif_ptr = (*ptr - '0') << 4;
> + break;
> + }
> + if (*ptr >= 'a' && *ptr <= 'f') {
> + *exif_ptr = (*ptr - 'a' + 10) << 4;
> + break;
> + }
> + }
> + while (++ptr < end) {
> + if (*ptr >= '0' && *ptr <= '9') {
> + *exif_ptr += *ptr - '0';
> + break;
> + }
> + if (*ptr >= 'a' && *ptr <= 'f') {
> + *exif_ptr += *ptr - 'a' + 10;
> + break;
> + }
> + }
> + if (ptr > end)
> + return AVERROR_INVALIDDATA;
> + exif_ptr++;
> + }
hex string parsing should be put in libavutil if we dont support it yet
Of course without the bugs (like the missing uppercase A-F support)
also the end handling is wrong
> +
> + return 0;
> +}
> +
> static int decode_text_chunk(PNGDecContext *s, GetByteContext *gb, int
> compressed)
> {
> int ret, method;
> @@ -582,6 +677,17 @@ static int decode_text_chunk(PNGDecContext *s,
> GetByteContext *gb, int compresse
> return AVERROR(ENOMEM);
> }
>
> + if (!strcmp(kw_utf8, "Raw profile type exif")) {
> + ret = decode_text_to_exif(s, txt_utf8);
> + if (ret < 0) {;
> + av_buffer_unref(&s->exif_data);
> + } else {
> + av_freep(&kw_utf8);
> + av_freep(&txt_utf8);
> + return ret;
> + }
> + }
> +
> av_dict_set(&s->frame_metadata, kw_utf8, txt_utf8,
> AV_DICT_DONT_STRDUP_KEY | AV_DICT_DONT_STRDUP_VAL);
> return 0;
> @@ -654,6 +760,23 @@ static int decode_phys_chunk(AVCodecContext *avctx,
> PNGDecContext *s,
> return 0;
> }
>
> +static int decode_exif_chunk(AVCodecContext *avctx, PNGDecContext *s,
> + GetByteContext *gb)
> +{
> + if (!(s->hdr_state & PNG_IHDR)) {
> + av_log(avctx, AV_LOG_ERROR, "eXIf before IHDR\n");
> + return AVERROR_INVALIDDATA;
> + }
This should check that there is only one eXIf and that its after IDAT as
required by the spec
(the code could ask for samples if there are multiple exif)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Some Animals are More Equal Than Others. - George Orwell's book Animal Farm
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
