On 10/19/2014 05:12 AM, Ray Strode wrote:
> From: Ray Strode <rstr...@redhat.com>
>
> 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 <a...@pobox.com>
>
> Signed-off-by: Ray Strode <rstr...@redhat.com>
> ---
> 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:
>
