]> git.tuebingen.mpg.de Git - paraslash.git/commit - aac_afh.c
mp4: Improve handling of read errors.
authorAndre Noll <maan@tuebingen.mpg.de>
Wed, 18 Aug 2021 13:25:17 +0000 (15:25 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Mon, 30 May 2022 19:37:35 +0000 (21:37 +0200)
commit5f123ac45aed8c3480e8211b513cf9b6bd7eb2ab
treecc8d29e08178072f05f7333b4c536c9da0fb0b3d
parenteb4b9e3d4f0c0c7e57057a0a3ce44bf8098d9464
mp4: Improve handling of read errors.

Currently read_data() of mp4.c is an atrocious mess. The ->read()
callback is defined to return uint32_t, but the return value is
stored in a signed 32 bit integer. Moreover, read_data() contains a
dead store, it handles neither short nor interrupted reads correctly,
and it moves the file position backwards on errors.

While this is easy to fix, a more intricate problem is that most
callers of read_data(), including all read_intX() helpers, ignore the
return value of read_data() and return uninitialized stack contents in
the error case. This is kind of dealt with by the ->read_error member
of struct mp4, but this not more than a kludge, which, according to
the comments, was applied after several CVEs had been filed against
the library.

Let's DTRT here, even though it adds a fair amount of new code:
Check the return value of each read operation and fail early on errors.

We have to distinguish three cases: error, EOF, and success, encoded
as return values -1, 0 and 1, respectively. This commit converts most
functions which read from an mp4 file to this convention. More work
is required as return values are not checked everywhere yet. This was
left for subsequent commits to keep the already large patch within
reasonable size.

Since we don't rely on ->read_error of struct mp4 any more, it can
be removed.
aac_afh.c
mp4.c
mp4.h