Patchwork ccid: Fix buffer overrun in handling of VSC_ATR message

login
register
mail settings
Submitter Markus Armbruster
Date Nov. 28, 2011, 7:27 p.m.
Message ID <1322508457-25520-1-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/128064/
State New
Headers show

Comments

Markus Armbruster - Nov. 28, 2011, 7:27 p.m.
ATR size exceeding the limit is diagnosed, but then we merrily use it
anyway, overrunning card->atr[].

The message is read from a character device.  Obvious security
implications unless the other end of the character device is trusted.

Spotted by Coverity.  CVE-2011-4111.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ccid-card-passthru.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Alon Levy - Nov. 29, 2011, 9:36 a.m.
On Mon, Nov 28, 2011 at 08:27:37PM +0100, Markus Armbruster wrote:
> ATR size exceeding the limit is diagnosed, but then we merrily use it
> anyway, overrunning card->atr[].
> 
> The message is read from a character device.  Obvious security
> implications unless the other end of the character device is trusted.
> 
> Spotted by Coverity.  CVE-2011-4111.
> 

Anthony, do you want me to do a pull request for this or can you merge
it as is?

Alon

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/ccid-card-passthru.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
> index 2cbc81b..9f51c6c 100644
> --- a/hw/ccid-card-passthru.c
> +++ b/hw/ccid-card-passthru.c
> @@ -150,6 +150,7 @@ static void ccid_card_vscard_handle_message(PassthruState *card,
>              error_report("ATR size exceeds spec, ignoring");
>              ccid_card_vscard_send_error(card, scr_msg_header->reader_id,
>                                          VSC_GENERAL_ERROR);
> +            break;
>          }
>          memcpy(card->atr, data, scr_msg_header->length);
>          card->atr_length = scr_msg_header->length;
> -- 
> 1.7.6.4
>
Markus Armbruster - Nov. 29, 2011, 9:51 a.m.
Alon Levy <alevy@redhat.com> writes:

> On Mon, Nov 28, 2011 at 08:27:37PM +0100, Markus Armbruster wrote:
>> ATR size exceeding the limit is diagnosed, but then we merrily use it
>> anyway, overrunning card->atr[].
>> 
>> The message is read from a character device.  Obvious security
>> implications unless the other end of the character device is trusted.
>> 
>> Spotted by Coverity.  CVE-2011-4111.
>> 
>
> Anthony, do you want me to do a pull request for this or can you merge
> it as is?

It's already in, commit 7e62255a.

Patch

diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c
index 2cbc81b..9f51c6c 100644
--- a/hw/ccid-card-passthru.c
+++ b/hw/ccid-card-passthru.c
@@ -150,6 +150,7 @@  static void ccid_card_vscard_handle_message(PassthruState *card,
             error_report("ATR size exceeds spec, ignoring");
             ccid_card_vscard_send_error(card, scr_msg_header->reader_id,
                                         VSC_GENERAL_ERROR);
+            break;
         }
         memcpy(card->atr, data, scr_msg_header->length);
         card->atr_length = scr_msg_header->length;