tpm2-software/tpm2-tss

SIGSEV on Fapi_GetAppData()

tswaehn opened this issue · 2 comments

I test the routines like this

  • get a random key
  • store random key as appData
  • get the appData => SIGSEV

code is as follows:

    // Generate random salt data
    uint8_t * random_data = nullptr;
    rc = Fapi_GetRandom(fapi_context, num_bytes, &random_data);
    if (rc != TSS2_RC_SUCCESS) {
        CHECK_FAPI_RESULT_AND_EXIT(rc, "Fapi_GetRandom");
    }

    // Set the random salt data (will be stored plaintext)
    rc = Fapi_SetAppData(fapi_context, key_path, (uint8_t*)random_data, num_bytes);
    if (rc != TSS2_RC_SUCCESS) {
        CHECK_FAPI_RESULT_AND_EXIT(rc, "Fapi_SetAppData");
    }

then get the appData

    uint32_t num_bytes = 0;
    uint8_t * app_data = nullptr;

    volatile uint64_t markerA= (uint64_t) fapi_context;

    rc = Fapi_GetAppData(fapi_context, key_path, &app_data, (size_t*) &num_bytes);
    if (rc != TSS2_RC_SUCCESS) {
        CHECK_FAPI_RESULT_AND_EXIT(rc, "Fapi_GetAppData()");
    }
    volatile uint64_t markerB= (uint64_t) fapi_context;

    if (markerA != markerB){
        exit(-1);
    }

it fails on markerA != markerB; that indicates Fapi_GetAppData() is modifying the fapi_context.

most likely I am doing something wrong. but I cant find it.

alright, I think I found it. Fapi_GetAppData() writes a 64bit number (size_t is probably 64bit).

uint32_t num_bytes = 0;

it should be (otherwise overwriting unrelated content of 32bits):

uint64_t num_bytes = 0;

I personally really dislike these non clear, kind of undefined or user defined types int, size_t, ...

suggestion:

  • we should replace size_t by uint64_t to make clear, how many bytes Fapi_GetAppData()will overwrite.

IMHO, size_t should be the standard-type for buffers on a given platform.
I mean, that's what it was intended for.
The problem is to cast the uint32_t* to a size_t* in your call. The compiler would have protected you from this.