Am Freitag, den 11.04.2014, 10:24 -0300 schrieb Richard: > Em 11-04-2014 06:45, Fuchs, Andreas escreveu: > > 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 > Yes, name is not used in both functions. Go figure... > > To be honest, I'm avoiding to make changes here as much as possible, > since I'm still investigating if this code should be part of TrouSerS. > It was created for use of IMA, but they don't use it that anymore, > according to one IMA developer I talked to. > > So, the idea was just to fix the issue as quickly as possible.
Well, I'd guess that the patch is correct, but I'd not know for sure... ;-) > > > > 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. > From my perspective, it doesn't expect \0. But since name is not used > anyway, doesn't matter. > > > > > 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
