diff mbox

[FFmpeg-devel] avutil/encryption_info: Don't pass NULL to memcpy

Message ID 20190918000358.25258-1-andreas.rheinhardt@gmail.com
State Accepted
Commit e6018fda14d7cfe2c890fb336c9264c4ea0b6c5c
Headers show

Commit Message

Andreas Rheinhardt Sept. 18, 2019, 12:03 a.m. UTC
The pointer arguments to memcpy (and several other functions of the
C standard library) are not allowed to be NULL, not even when the number
of bytes to copy is zero. An AVEncryptionInitInfo's data pointer is
explicitly allowed to be NULL and yet av_encryption_init_info_add_side_data
unconditionally used it as a source pointer to copy from. This commit changes
this so that copying is only done if the number of bytes to copy is > 0.

Fixes ticket #8141 as well as a part of ticket #8150.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
This is a slightly improved version (see the code). Furthermore, a typo
in the commit message has been fixed.

 libavutil/encryption_info.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Tomas Härdin Sept. 18, 2019, 6:20 p.m. UTC | #1
ons 2019-09-18 klockan 02:03 +0200 skrev Andreas Rheinhardt:
> The pointer arguments to memcpy (and several other functions of the
> C standard library) are not allowed to be NULL, not even when the number
> of bytes to copy is zero. An AVEncryptionInitInfo's data pointer is
> explicitly allowed to be NULL and yet av_encryption_init_info_add_side_data
> unconditionally used it as a source pointer to copy from. This commit changes
> this so that copying is only done if the number of bytes to copy is > 0.

*gesticulates wildly with a formal verification stick*

> -        memcpy(cur_buffer, cur_info->data, cur_info->data_size);
> -        cur_buffer += cur_info->data_size;
> +        if (cur_info->data_size > 0) {
> +            memcpy(cur_buffer, cur_info->data, cur_info->data_size);
> +            cur_buffer += cur_info->data_size;
> +        }

Approve

There's no doubt more of these

/Tomas
Michael Niedermayer Sept. 20, 2019, 9:16 p.m. UTC | #2
On Wed, Sep 18, 2019 at 08:20:10PM +0200, Tomas Härdin wrote:
> ons 2019-09-18 klockan 02:03 +0200 skrev Andreas Rheinhardt:
> > The pointer arguments to memcpy (and several other functions of the
> > C standard library) are not allowed to be NULL, not even when the number
> > of bytes to copy is zero. An AVEncryptionInitInfo's data pointer is
> > explicitly allowed to be NULL and yet av_encryption_init_info_add_side_data
> > unconditionally used it as a source pointer to copy from. This commit changes
> > this so that copying is only done if the number of bytes to copy is > 0.
> 
> *gesticulates wildly with a formal verification stick*
> 
> > -        memcpy(cur_buffer, cur_info->data, cur_info->data_size);
> > -        cur_buffer += cur_info->data_size;
> > +        if (cur_info->data_size > 0) {
> > +            memcpy(cur_buffer, cur_info->data, cur_info->data_size);
> > +            cur_buffer += cur_info->data_size;
> > +        }
> 
> Approve

will apply

thx

[...]
diff mbox

Patch

diff --git a/libavutil/encryption_info.c b/libavutil/encryption_info.c
index 812c704776..dd3fa71a44 100644
--- a/libavutil/encryption_info.c
+++ b/libavutil/encryption_info.c
@@ -331,8 +331,10 @@  uint8_t *av_encryption_init_info_add_side_data(const AVEncryptionInitInfo *info,
             memcpy(cur_buffer, cur_info->key_ids[i], cur_info->key_id_size);
             cur_buffer += cur_info->key_id_size;
         }
-        memcpy(cur_buffer, cur_info->data, cur_info->data_size);
-        cur_buffer += cur_info->data_size;
+        if (cur_info->data_size > 0) {
+            memcpy(cur_buffer, cur_info->data, cur_info->data_size);
+            cur_buffer += cur_info->data_size;
+        }
     }
 
     return buffer;