[libvirt] [PATCH 6/8] virTestCompareToFile: Don't access memory we don't own

Michal Privoznik posted 8 patches 7 years, 9 months ago
[libvirt] [PATCH 6/8] virTestCompareToFile: Don't access memory we don't own
Posted by Michal Privoznik 7 years, 9 months ago
After reading the contents of a file some cleanup is performed.
However, the check for it might access a byte outside of the
string - if the file is empty in the first place. Then strlen()
is zero.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/testutils.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/testutils.c b/tests/testutils.c
index 71692f1fa..4bb6140ec 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -796,6 +796,7 @@ virTestCompareToFile(const char *strcontent,
         goto failure;
 
     if (filecontent &&
+        strlen(filecontent) > 0 &&
         filecontent[strlen(filecontent) - 1] == '\n' &&
         strcontent[strlen(strcontent) - 1] != '\n') {
         if (virAsprintf(&fixedcontent, "%s\n", strcontent) < 0)
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] virTestCompareToFile: Don't access memory we don't own
Posted by Peter Krempa 7 years, 9 months ago
On Fri, Aug 04, 2017 at 16:22:34 +0200, Michal Privoznik wrote:
> After reading the contents of a file some cleanup is performed.
> However, the check for it might access a byte outside of the
> string - if the file is empty in the first place. Then strlen()
> is zero.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tests/testutils.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/testutils.c b/tests/testutils.c
> index 71692f1fa..4bb6140ec 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -796,6 +796,7 @@ virTestCompareToFile(const char *strcontent,
>          goto failure;
>  
>      if (filecontent &&
> +        strlen(filecontent) > 0 &&

I'd store the length in a variable ...

>          filecontent[strlen(filecontent) - 1] == '\n' &&

... so that it's not evaluated twice.

>          strcontent[strlen(strcontent) - 1] != '\n') {
>          if (virAsprintf(&fixedcontent, "%s\n", strcontent) < 0)

ACK with that.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list