qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] libcacard: don't free sign buffer while sig


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 3/3] libcacard: don't free sign buffer while sign op is pending
Date: Mon, 20 Oct 2014 20:40:43 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0



On 10/19/2014 08:23 AM, Alon Levy wrote:
On 10/19/2014 05:12 AM, Ray Strode wrote:
From: Ray Strode <address@hidden>

commit 57f97834efe0c208ffadc9d2959f3d3d55580e52 cleaned up
the cac_applet_pki_process_apdu function to have a single
exit point. Unfortunately, that commit introduced a bug
where the sign buffer can get free'd and nullified while
it's still being used.

This commit corrects the bug by introducing a boolean to
track whether or not the sign buffer should be freed in
the function exit path.

My bad, thanks for catching this.

Reviewed-by: Alon Levy <address@hidden>

Please Cc address@hidden if you send a pull request.

Paolo



Signed-off-by: Ray Strode <address@hidden>
---
  libcacard/cac.c | 10 +++++++---
  1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libcacard/cac.c b/libcacard/cac.c
index ae8c378..f38fdce 100644
--- a/libcacard/cac.c
+++ b/libcacard/cac.c
@@ -88,60 +88,61 @@ cac_common_process_apdu(VCard *card, VCardAPDU *apdu, 
VCardResponse **response)
  }

  /*
   *  reset the inter call state between applet selects
   */
  static VCardStatus
  cac_applet_pki_reset(VCard *card, int channel)
  {
      VCardAppletPrivate *applet_private;
      CACPKIAppletData *pki_applet;
      applet_private = vcard_get_current_applet_private(card, channel);
      assert(applet_private);
      pki_applet = &(applet_private->u.pki_data);

      pki_applet->cert_buffer = NULL;
      g_free(pki_applet->sign_buffer);
      pki_applet->sign_buffer = NULL;
      pki_applet->cert_buffer_len = 0;
      pki_applet->sign_buffer_len = 0;
      return VCARD_DONE;
  }

  static VCardStatus
  cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
                              VCardResponse **response)
  {
      CACPKIAppletData *pki_applet;
      VCardAppletPrivate *applet_private;
      int size, next;
      unsigned char *sign_buffer;
+    bool retain_sign_buffer = FALSE;
      vcard_7816_status_t status;
      VCardStatus ret = VCARD_FAIL;

      applet_private = vcard_get_current_applet_private(card, apdu->a_channel);
      assert(applet_private);
      pki_applet = &(applet_private->u.pki_data);

      switch (apdu->a_ins) {
      case CAC_UPDATE_BUFFER:
          *response = vcard_make_response(
              VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED);
          ret = VCARD_DONE;
          break;
      case CAC_GET_CERTIFICATE:
          if ((apdu->a_p2 != 0) || (apdu->a_p1 != 0)) {
              *response = vcard_make_response(
                               VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
              break;
          }
          assert(pki_applet->cert != NULL);
          size = apdu->a_Le;
          if (pki_applet->cert_buffer == NULL) {
              pki_applet->cert_buffer = pki_applet->cert;
              pki_applet->cert_buffer_len = pki_applet->cert_len;
          }
          size = MIN(size, pki_applet->cert_buffer_len);
          next = MIN(255, pki_applet->cert_buffer_len - size);
          *response = vcard_response_new_bytes(
                          card, pki_applet->cert_buffer, size,
                          apdu->a_Le, next ?
@@ -151,85 +152,88 @@ cac_applet_pki_process_apdu(VCard *card, VCardAPDU *apdu,
          pki_applet->cert_buffer += size;
          pki_applet->cert_buffer_len -= size;
          if ((*response == NULL) || (next == 0)) {
              pki_applet->cert_buffer = NULL;
          }
          if (*response == NULL) {
              *response = vcard_make_response(
                              VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
          }
          ret = VCARD_DONE;
          break;
      case CAC_SIGN_DECRYPT:
          if (apdu->a_p2 != 0) {
              *response = vcard_make_response(
                               VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
              break;
          }
          size = apdu->a_Lc;

          sign_buffer = g_realloc(pki_applet->sign_buffer,
                                  pki_applet->sign_buffer_len + size);
          memcpy(sign_buffer+pki_applet->sign_buffer_len, apdu->a_body, size);
          size += pki_applet->sign_buffer_len;
          switch (apdu->a_p1) {
          case  0x80:
              /* p1 == 0x80 means we haven't yet sent the whole buffer, wait for
               * the rest */
              pki_applet->sign_buffer = sign_buffer;
              pki_applet->sign_buffer_len = size;
              *response = vcard_make_response(VCARD7816_STATUS_SUCCESS);
+            retain_sign_buffer = TRUE;
              break;
          case 0x00:
              /* we now have the whole buffer, do the operation, result will be
               * in the sign_buffer */
              status = vcard_emul_rsa_op(card, pki_applet->key,
                                         sign_buffer, size);
              if (status != VCARD7816_STATUS_SUCCESS) {
                  *response = vcard_make_response(status);
                  break;
              }
              *response = vcard_response_new(card, sign_buffer, size, 
apdu->a_Le,
                                                       
VCARD7816_STATUS_SUCCESS);
              if (*response == NULL) {
                  *response = vcard_make_response(
                                  VCARD7816_STATUS_EXC_ERROR_MEMORY_FAILURE);
              }
              break;
          default:
             *response = vcard_make_response(
                                  VCARD7816_STATUS_ERROR_P1_P2_INCORRECT);
              break;
          }
-        g_free(sign_buffer);
-        pki_applet->sign_buffer = NULL;
-        pki_applet->sign_buffer_len = 0;
+        if (!retain_sign_buffer) {
+            g_free(sign_buffer);
+            pki_applet->sign_buffer = NULL;
+            pki_applet->sign_buffer_len = 0;
+        }
          ret = VCARD_DONE;
          break;
      case CAC_READ_BUFFER:
          /* new CAC call, go ahead and use the old version for now */
          /* TODO: implement */
          *response = vcard_make_response(
                                  VCARD7816_STATUS_ERROR_COMMAND_NOT_SUPPORTED);
          ret = VCARD_DONE;
          break;
      default:
          ret = cac_common_process_apdu(card, apdu, response);
          break;
      }
      return ret;
  }


  static VCardStatus
  cac_applet_id_process_apdu(VCard *card, VCardAPDU *apdu,
                             VCardResponse **response)
  {
      VCardStatus ret = VCARD_FAIL;

      switch (apdu->a_ins) {
      case CAC_UPDATE_BUFFER:
          *response = vcard_make_response(
                          VCARD7816_STATUS_ERROR_CONDITION_NOT_SATISFIED);
          ret = VCARD_DONE;
          break;
      case CAC_READ_BUFFER:







reply via email to

[Prev in Thread] Current Thread [Next in Thread]