All,
While doing a routine update of a security review of the tboot code, I
found a couple of minor problems - two potential (but very unlikely)
buffer overrun problems, and one minor memory leak - although the
program is going to terminate almost immediately, so the memory comes
back anyway.
At any rate, here is a patch to correct the problem.
Signed Off by: Charles Fisher charles.fis...@gdc4s.com
diff -up tboot-1.7.2/lcptools/crtpconf.c.orig
tboot-1.7.2/lcptools/crtpconf.c
--- tboot-1.7.2/lcptools/crtpconf.c.orig 2012-12-11 13:16:12.239464000
-0700
+++ tboot-1.7.2/lcptools/crtpconf.c 2012-12-11 16:00:23.097982000 -0700
@@ -109,14 +109,12 @@ main(int argc, char *argv[])
uint16_t i = 0;
uint32_t index[MAX_INDEX] = {0};
uint32_t idx_num = 0;
- unsigned char *pcr_num[MAX_INDEX] = {NULL};
FILE *p_file = NULL;
unsigned char* srtm_data = NULL;
uint32_t data_len = 0;
TPM_LOCALITY_SELECTION local_sel;
lcp_result_t ret_value = LCP_E_COMD_INTERNAL_ERR;
- uint32_t temp = 0;
/*
* No parameter input will print out the help message.
@@ -151,28 +149,13 @@ main(int argc, char *argv[])
ret_value = LCP_E_INVALID_PARAMETER;
goto _error_end;
}
-
- for (i = 0; i < MAX_INDEX; i++) {
- pcr_num[i] = (unsigned char *)malloc(10);
- if ( pcr_num[i] == NULL ) {
- ret_value = LCP_E_OUTOFMEMORY;
- goto _error_end;
- }
- }
- if ( str_split((char *)pcr_val, (char **)&pcr_num, &idx_num) < 0 )
{
- ret_value = LCP_E_INVALID_PARAMETER;
- goto _error_end;
- }
+ idx_num = MAX_INDEX;
+ str_split((char *)pcr_val, index, &idx_num);
for ( i = 0; i < idx_num; i++ ) {
- if ( strtonum((char *)pcr_num[i], &temp) < 0 ) {
- ret_value = LCP_E_INVALID_PARAMETER;
- goto _error_end;
- }
- if ( temp > 23 ) {
+ if ( index[i] > 23 ) {
ret_value = LCP_E_INVALID_PARAMETER;
goto _error_end;
- }
- index[i] = temp;
+ }
}
local_sel = (TPM_LOCALITY_SELECTION)locality;
@@ -200,8 +183,7 @@ main(int argc, char *argv[])
fclose(p_file);
} else
print_hexmsg("the PConf data is:\n", data_len, srtm_data);
- if(srtm_data)
- free(srtm_data);
+ free(srtm_data);
} else
goto _error_end;
@@ -210,10 +192,10 @@ _error_end:
/*
* Error when execute.
*/
- for (i = 0; i < MAX_INDEX; i++)
- free(pcr_num[i]);
- free(srtm_data);
+ if (srtm_data)
+ free(srtm_data);
log_error("\nCommand CrtPConf failed:\n");
print_error(ret_value);
return ret_value;
-}
+ }
+
diff -up tboot-1.7.2/lcptools/lcputils.c.orig
tboot-1.7.2/lcptools/lcputils.c
--- tboot-1.7.2/lcptools/lcputils.c.orig 2012-12-11 13:16:30.352217000
-0700
+++ tboot-1.7.2/lcptools/lcputils.c 2012-12-11 15:44:03.076312000 -0700
@@ -217,42 +217,22 @@ print_hexmsg(const char *header_msg, int
}
/* split the input string in the format: num1,num2,...,numN
- * into the array = {num1, num2, ... , numN}
+ * into the numeric array = {num1, num2, ... , numN}
*/
-int
-str_split(const char *str_in, char **str_out, unsigned int *number)
+void
+str_split(char *str_in, uint32_t ints[], unsigned int *nr_ints)
{
- char * temp;
- int num = 0;
- const char *sep = ",";
- size_t str_length = 0;
- char *string = (char *)malloc(strlen(str_in) + 1);
-
- if ( string == NULL )
- return -1;
- if ( str_in == NULL || str_out == NULL || number == NULL ) {
- free(string);
- return -1;
- }
- strcpy(string, str_in);
- temp =strtok(string, sep);
- if ( temp != NULL && str_out[num] )
- strcpy(str_out[num], temp);//strtok(string, sep));
- while (str_out[num] != NULL) {
- str_length += strlen(str_out[num]);
- num++;
- temp = strtok(NULL, sep);
- if ( temp != NULL )
- strcpy(str_out[num], temp);
- else
- str_out[num] = NULL;
+ unsigned int nr = 0;
+
+ while ( true ) {
+ char *str = strsep(&str_in, ",");
+ if ( str == NULL || nr == *nr_ints )
+ break;
+ ints[nr++] = strtoul(str, NULL, 0);
}
- free(string);
- *number = num;
- str_length += num - 1;
- if ( str_length != strlen(str_in) )
- return -1;
- return 0;
+ if ( nr == *nr_ints )
+ log_error("Error: too many items in list\n");
+ *nr_ints = nr;
}
uint16_t
diff -up tboot-1.7.2/lcptools/lcputils.h.orig
tboot-1.7.2/lcptools/lcputils.h
--- tboot-1.7.2/lcptools/lcputils.h.orig 2012-12-11 15:20:08.106747000
-0700
+++ tboot-1.7.2/lcptools/lcputils.h 2012-12-11 15:42:34.009610000 -0700
@@ -134,6 +134,6 @@ calc_sizeofselect(uint32_t num_indices,
void print_locality(unsigned char loc);
void print_permissions(UINT32 perms, const char *prefix);
-int str_split(const char *str_in, char **str_out, unsigned int
*number);
+void str_split(char *str_in, uint32_t ints[], unsigned int *number);
#endif
diff -up tboot-1.7.2/lcptools/lock.c.orig tboot-1.7.2/lcptools/lock.c
--- tboot-1.7.2/lcptools/lock.c.orig 2012-12-11 14:57:02.784235000
-0700
+++ tboot-1.7.2/lcptools/lock.c 2012-12-11 15:15:43.532763000 -0700
@@ -91,7 +91,8 @@ parse_cmdline(int argc, const char * arg
int
main (int argc, char *argv[])
{
- char confirm_lock[1024] = {0};
+ char confirm_lock[4] = {0};
+ char c;
in_nv_definespace_t in_defspace;
lcp_result_t ret_value = LCP_E_COMD_INTERNAL_ERR;
@@ -119,12 +120,12 @@ main (int argc, char *argv[])
*/
do {
log_info("Really want to lock TPM NV? (Y/N) ");
- dummy = scanf("%s", confirm_lock);
+ dummy = scanf("%3s", confirm_lock);
if ( dummy <= 0 )
return LCP_E_COMD_INTERNAL_ERR;
- } while (strcmp(confirm_lock, "N") && strcmp(confirm_lock, "n")
&&
- strcmp(confirm_lock, "Y") && strcmp(confirm_lock, "y"));
- if ( !strcmp(confirm_lock, "N") || !strcmp(confirm_lock, "n") )
{
+ c = confirm_lock[0] | ' ';
+ } while ( (c != 'n') && (c != 'y') );
+ if ( c == 'n') {
ret_value = LCP_SUCCESS;
return ret_value;
}
------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
tboot-devel mailing list
tboot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tboot-devel