Skip to contentSkip to conversation

crypto: fix copy size causing overruns/crashing

PATCH In review Will Dietz 3 participants 5 messages Oct 3, 2019

lore
Share & tools
lore

Will Dietz

author
#0 lore raw

PATCH 1/1 crypto: fix copy size causing overruns/crashing

num_ad is already accounted for in sizeof(iov) as iov has size sizeof(struct iovec) * (num_ad+1).

---
 src/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/crypto.c +2 -2
index 632117d..62edd44 100644
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
        struct iovec iov[num_ad + 1];
        uint8_t v[16];
 
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
        iov[num_ad].iov_base = (void *)in;
        iov[num_ad].iov_len = in_len;
        num_ad++;

@@ -368,7 +368,7 @@ bool aes_siv_decrypt(const uint8_t *key, size_t key_len, const uint8_t *in,
        if (in_len < 16)
                return false;
 
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);
        iov[num_ad].iov_base = (void *)out;
        iov[num_ad].iov_len = in_len - 16;
        num_ad++;
Signature · 1 line
2.23.GIT
#1 lore raw

Hi Will,

Quoted 11 lines · Will Dietz

num_ad is already accounted for in sizeof(iov) as iov has size sizeof(struct iovec) * (num_ad+1).

---
 src/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/crypto.c +0 -0
index 632117d..62edd44 100644

src/crypto.c bool aes_siv_encrypt(const uint8_t *key, size_t · line 315

Quoted 6 lines
src/crypto.c +1 -1
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
 key_len, const uint8_t *in,
        struct iovec iov[num_ad + 1];
        uint8_t v[16];
 
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);

Good catch! I'm fine with this way, but also fine with simply removing
* num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.

Thanks,
James

Quoted 3 lines

_______________________________________________
iwd mailing list -- iwd(a)lists.01.org
To unsubscribe send an email to iwd-leave(a)lists.01.org

#2 lore raw
Quoted 18 lines · James

Hi Will,

Quoted 15 lines · Will Dietz

num_ad is already accounted for in sizeof(iov) as iov has size sizeof(struct iovec) * (num_ad+1).

---
 src/crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/crypto.c +0 -0
index 632117d..62edd44 100644
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
 key_len, const uint8_t *in,
        struct iovec iov[num_ad + 1];
        uint8_t v[16];
Quoted 6 lines

src/crypto.c bool aes_siv_encrypt(const uint8_t *key, size_t · line 315

Quoted 2 lines
src/crypto.c +1 -1
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);

Good catch! I'm fine with this way, but also fine with simply removing
* num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.

Oh sorry, this wont work. That would copy past 'ad'. Nevermind, lets do it your way ;)

Quoted 10 lines

Thanks,
James

Quoted 3 lines

_______________________________________________
iwd mailing list -- iwd(a)lists.01.org
To unsubscribe send an email to iwd-leave(a)lists.01.org

_______________________________________________
iwd mailing list -- iwd(a)lists.01.org
To unsubscribe send an email to iwd-leave(a)lists.01.org

#3 lore raw
In reply to Prestwood

Hi James,

Quoted 18 lines · James

Hi Will,

Quoted 15 lines · Will Dietz

num_ad is already accounted for in sizeof(iov) as iov has size sizeof(struct iovec) * (num_ad+1).

---
  src/crypto.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
src/crypto.c +0 -0
index 632117d..62edd44 100644
@@ -311,7 +311,7 @@ bool aes_siv_encrypt(const uint8_t *key, size_t
 key_len, const uint8_t *in,
         struct iovec iov[num_ad + 1];
         uint8_t v[16];
Quoted 6 lines

src/crypto.c bool aes_siv_encrypt(const uint8_t *key, size_t · line 315

Quoted 2 lines
src/crypto.c +1 -1
-       memcpy(iov, ad, sizeof(iov) * num_ad);
+       memcpy(iov, ad, sizeof(struct iovec) * num_ad);

Good catch! I'm fine with this way, but also fine with simply removing
* num_ad and leaving sizeof(iov) as is. Lets see what Denis prefers.

Not sure that will work since you define iov to be an array of size num_ad + 1?

Regards,
-Denis

#4 lore raw
In reply to Will Dietz

Hi Will,

Quoted 6 lines · Will Dietz

num_ad is already accounted for in sizeof(iov) as iov has size sizeof(struct iovec) * (num_ad+1).

---
  src/crypto.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

Can you use git send-email to submit this? When I save this message it comes in as multipart with the signature, etc included and seems to confuse git am.

Regards,
-Denis