On Tue, 31 Aug 2010 10:44:40 -0500 Kent Yoder <[email protected]> wrote:
> The following patches close several holes in openCryptoki's parsing of token > data and correct NULL pointer deferences of the pValue attribue of some > templates. > > Signed-off-by: Kent Yoder <[email protected]> I have intentions of applying this patch as-is, but I have a few comments anyway (with the hope we could address those in the short future): * the public and private object loading functions are using PATH_MAX for the file names but reading a harcoded "50" bytes for the filename in the index file? * We're not checking the return value for fread() when dealing with the size and "is_private" for the objects. Corruption could, in theory, happen in those two first fields as well, right? * Is it a good idea mixing up CK_ULONG_32 size from the object loading functions with a simple "int" from the *_withSize() functions? Should we be using arch-independent types and perhaps defining max object sizes? * Perhaps we could define a new "Token object corrupted" standard message for st_err_log() and use that instead? In fact I hate how we have st_err_log(), LOG(), and several other things for the same purpose. I saw we are including <syslog.h> for the sole purpose of using LOG_ERR also. I won't mind since I know we have been planning ontemplate_unflatten_withSize refactoring this area anyway. Maybe we could take the same approach as we did in the testcases? i.e., #define LOG_ERR(msgcode_enum, fmt_string, ...) same for LOG_WARN(), LOG_DEBUG(), perhaps even a special LOG_TRACE()... * I don't think we use CamelCase anywhere else in the code. * I saw we are checking for "NULL" pValues in attributes, but where are NULL values assigned if they are found to be corrupted to begin with? Seems like template_unflatten_withSize() is only assigning pValue with NULL if ulValueLen is zero, a check that was being made anyway. -Klaus -- Klaus Heinrich Kiwi | [email protected] | http://blog.klauskiwi.com Open Source Security blog : http://www.ratliff.net/blog IBM Linux Technology Center : http://www.ibm.com/linux/ltc ------------------------------------------------------------------------------ This SF.net Dev2Dev email is sponsored by: Show off your parallel programming skills. Enter the Intel(R) Threading Challenge 2010. http://p.sf.net/sfu/intel-thread-sfd _______________________________________________ Opencryptoki-tech mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opencryptoki-tech
