qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	qemu-block@nongnu.org,
	"Johannes Schindelin" <johannes.schindelin@gmx.de>,
	"Max Reitz" <mreitz@redhat.com>
Subject: [PATCH 4/4] block/vvfat: Avoid out of bounds write in create_long_filename()
Date: Fri, 30 Apr 2021 18:25:19 +0200	[thread overview]
Message-ID: <20210430162519.271607-5-philmd@redhat.com> (raw)
In-Reply-To: <20210430162519.271607-1-philmd@redhat.com>

The direntry_t::name holds 11 bytes:

  typedef struct direntry_t {
      uint8_t name[8 + 3];
      ...

However create_long_filename() writes up to 31 bytes into it:

 421     for(i=0;i<26*number_of_entries;i++) {
 422         int offset=(i%26);
 423         if(offset<10) offset=1+offset;
 424         else if(offset<22) offset=14+offset-10;
 425         else offset=28+offset-22;
 426         entry=array_get(&(s->directory),s->directory.next-1-(i/26));
 427         if (i >= 2 * length + 2) {
 428             entry->name[offset] = 0xff;
 429         } else if (i % 2 == 0) {
 430             entry->name[offset] = longname[i / 2] & 0xff;
 431         } else {
 432             entry->name[offset] = longname[i / 2] >> 8;
 433         }
 434     }

For example, if i=25, offset=28+25-22=31

Then in lines 428, 430 and 432 the entry->name[] array is written beside
its 11 bytes, as reported by Clang sanitizer:

  block/vvfat.c:430:13: runtime error: index 14 out of bounds for type 'uint8_t [11]'
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior block/vvfat.c:430:13 in
  block/vvfat.c:432:13: runtime error: index 15 out of bounds for type 'uint8_t [11]'
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior block/vvfat.c:432:13 in
  block/vvfat.c:428:13: runtime error: index 18 out of bounds for type 'uint8_t [11]'
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior block/vvfat.c:428:13 in

As I have no idea about what this code does, simply skip the writes if
out of range, since it is not worst than what we have currently (and
my tests using vvfat work identically).

Fixes: de167e416fa ("Virtual VFAT support (Johannes Schindelin)")
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 block/vvfat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/vvfat.c b/block/vvfat.c
index c193a816646..c7162e77d68 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -423,6 +423,9 @@ static direntry_t *create_long_filename(BDRVVVFATState *s, const char *filename)
         if(offset<10) offset=1+offset;
         else if(offset<22) offset=14+offset-10;
         else offset=28+offset-22;
+        if (offset >= ARRAY_SIZE(entry->name)) {
+            continue;
+        }
         entry=array_get(&(s->directory),s->directory.next-1-(i/26));
         if (i >= 2 * length + 2) {
             entry->name[offset] = 0xff;
-- 
2.26.3



  parent reply	other threads:[~2021-04-30 16:47 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 16:25 [PATCH 0/4] block/vvfat: Fix leaks and out of bounds writes Philippe Mathieu-Daudé
2021-04-30 16:25 ` [PATCH 1/4] block/vvfat: Fix leak of BDRVVVFATState::qcow_filename Philippe Mathieu-Daudé
2021-05-03 13:14   ` Stefano Garzarella
2021-04-30 16:25 ` [PATCH 2/4] block/vvfat: Fix leak of BDRVVVFATState::used_clusters Philippe Mathieu-Daudé
2021-05-03 13:24   ` Stefano Garzarella
2021-04-30 16:25 ` [PATCH 3/4] block/vvfat: Fix leak of mapping_t::path Philippe Mathieu-Daudé
2021-05-07 16:17   ` Max Reitz
2021-04-30 16:25 ` Philippe Mathieu-Daudé [this message]
2021-05-05 13:46   ` [PATCH 4/4] block/vvfat: Avoid out of bounds write in create_long_filename() Eric Blake
2021-05-05 13:11 ` [PATCH 0/4] block/vvfat: Fix leaks and out of bounds writes Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210430162519.271607-5-philmd@redhat.com \
    --to=philmd@redhat.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).