Disclaimer: I could not complie-test or runtime-test these patches right now. This is a pure code-only review of the patches.
inside ima_get_entries_by_pcr() name is unused... ?!?!? maybe the fread() should be replaced with an fseek(fp, len, SEEK_CUR) in this case. However the length check would make sense non-the-less I guess. Same for digest. I'd also externalize the 20 for DIGEST_SIZE similar to FILENAME_MAXSIZE inside ima_get_entry() the same is true. Also, since I don't know, how name is used, I am not sure, if it needs to be '\0'-terminated. If so, then the check would need to be (len > MAX_LEN - 1). Sorry, I cannot say anything about this patch, since I don't understand the functions. Am Mittwoch, den 09.04.2014, 15:41 -0300 schrieb [email protected]: > From: Richard Maciel <[email protected]> > > Since the size of the name could be read from a file, but the buffer > to contain it was fixed size, a check was needed to ensure that > the fread doesn't overrun the buffer. > > Signed-off-by: Richard Maciel <[email protected]> > --- > src/tcs/tcs_evlog_imaem.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/src/tcs/tcs_evlog_imaem.c b/src/tcs/tcs_evlog_imaem.c > index 1771dbc..d905381 100644 > --- a/src/tcs/tcs_evlog_imaem.c > +++ b/src/tcs/tcs_evlog_imaem.c > @@ -50,6 +50,8 @@ > > #ifdef EVLOG_SOURCE_IMA > > +#define EVLOG_FILENAME_MAXSIZE 255 > + > struct ext_log_source ima_source = { > ima_open, > ima_get_entries_by_pcr, > @@ -84,7 +86,7 @@ ima_get_entries_by_pcr(FILE *handle, UINT32 pcr_index, > UINT32 first, > TSS_RESULT result = TCSERR(TSS_E_INTERNAL_ERROR); > FILE *fp = (FILE *) handle; > uint len; > - char name[255]; > + char name[EVLOG_FILENAME_MAXSIZE]; > > if (!fp) { > LogError("File handle is NULL!\n"); > @@ -132,8 +134,12 @@ ima_get_entries_by_pcr(FILE *handle, UINT32 pcr_index, > UINT32 first, > result = TCSERR(TSS_E_INTERNAL_ERROR); > goto free_list; > } > - > - memset(name, 0, sizeof name); > + if (len > EVLOG_FILENAME_MAXSIZE) { > + LogError("Event log file name too big! Max size is %d", > EVLOG_FILENAME_MAXSIZE); > + result = TCSERR(TSS_E_INTERNAL_ERROR); > + goto free_list; > + } > + memset(name, 0, EVLOG_FILENAME_MAXSIZE); > if (fread(name, 1, len, fp) != len) { > LogError("Failed to read event log file"); > result = TCSERR(TSS_E_INTERNAL_ERROR); > @@ -229,7 +235,7 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 > *num, TSS_PCR_EVENT **ppEve > TSS_RESULT result = TCSERR(TSS_E_INTERNAL_ERROR); > TSS_PCR_EVENT *event = NULL; > FILE *fp = (FILE *) handle; > - char name[255]; > + char name[EVLOG_FILENAME_MAXSIZE]; > > rewind(fp); > while (fread(page, 24, 1, fp)) { > @@ -269,7 +275,13 @@ ima_get_entry(FILE *handle, UINT32 pcr_index, UINT32 > *num, TSS_PCR_EVENT **ppEve > result = > TCSERR(TSS_E_INTERNAL_ERROR); > goto done; > } > - memset(name, 0, sizeof name); > + if (len > EVLOG_FILENAME_MAXSIZE) { > + free(event); > + LogError("Event log file name > too big! Max size is %d", EVLOG_FILENAME_MAXSIZE); > + result = > TCSERR(TSS_E_INTERNAL_ERROR); > + goto done; > + } > + memset(name, 0, EVLOG_FILENAME_MAXSIZE); > if (fread(name, 1, len, fp) != len) { > free(event); > LogError("Failed to read event > log file"); ------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees _______________________________________________ TrouSerS-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/trousers-tech
