Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAPI] Fix appdata corruption caused by offset miscalculation #859

Merged

Conversation

wxleong
Copy link
Member

@wxleong wxleong commented Apr 22, 2024

When the FAPI backend is used, the following steps occur whenever C_SetAttributeValue() is invoked to update attributes. The function backend_fapi_update_tobject_attrs() performs these actions:

  1. Invoke Fapi_GetAppData() to retrieve the existing appdata.
  2. Find the offset (tobj_start) of the tobj to be deleted in the appdata.
  3. Allocate a new buffer newappdata.
  4. Copy only the data before tobj_start from appdata to newappdata:
    memcpy(&newappdata[0], &appdata[0], tobj_start);
    
  5. Append the new value (attrs) to newappdata:
    sprintf((char*)&newappdata[tobj_start], "%08x:%s", tobj->id, attrs);
    
  6. Append the remaining data from appdata:
    memcpy(&newappdata[tobj_start + 9 + strlen(attrs) + 1],
           &appdata[tobj_start + tobj_len], <======================= [Issue]
           appdata_len - tobj_start - tobj_len - 1);
    
  7. Call Fapi_SetAppData(newappdata) to update the data.

[Issue]: It does not account for the '\0' character. Essentially, the '\0' that marks the end of a tobj is copied over, resulting in the next tobj starting with a '\0' character, making subsequent tobjs unrecognizable.

@AndreasFuchsTPM AndreasFuchsTPM merged commit f85d141 into tpm2-software:master May 2, 2024
13 checks passed
AndreasFuchsTPM pushed a commit that referenced this pull request May 6, 2024
@wxleong wxleong deleted the fix-corrupted-appdata branch May 14, 2024 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants