Re: PKCS11 Extension

Hi Guillaume, same comments than Johannes, ecalloc()/efree()/emalloc() should be used. Since they are many crypto sensitive contents, only ecalloc() should be used (no emalloc()). Regarding the Oasis' semantic, I feel that it is important to keep it "as is" so the vanilla documentation would be the upstream Oasis PKCS11 standard.. It means, for instance: getInfo() -> C_GetInfo() It is nice to have some sign() methods in order to avoid combining C_SignInit() + C_Sign(), but they are some cases when we need to stick with the Oasis' specifications, so I believe that C_SignInit() C_VerifyInit() C_FindObjectsInit() C_FindObjectsFinal() etc. should still be exposed per the Oasis' specification, along having some nice sign(), verify(), etc. methods too. Regarding: C_GetAttributeValue() it has many "OK" cases: See Oasis' comments: "Note that the error codes CKR_ATTRIBUTE_SENSITIVE, CKR_ATTRIBUTE_TYPE_INVALID, and CKR_BUFFER_TOO_SMALL do not denote true errors for C_GetAttributeValue. If a call to C_GetAttributeValue returns any of these three values, then the call MUST nonetheless have processed every attribute in the template supplied to C_GetAttributeValue. Each attribute in the template whose value can be returned by the call to C_GetAttributeValue will be returned by the call to C_GetAttributeValue." my 2 cents, Vincent ----- Mail original ----- De: "Johannes Schlüter" <johannes@schlueters.de> À: "Guillaume Amringer" <g.amringer@gmail.com>, "pecl-dev" <pecl-dev@lists.php.net> Envoyé: Lundi 20 Juillet 2020 14:55:33 Objet: Re: [PECL-DEV] PKCS11 Extension Hi, On Sun, 2020-07-19 at 22:33 -0400, Guillaume Amringer wrote: > The extension is currently hosted here: > https://github.com/gamringer/php-pkcs11 I scrolled over it without much attention (thus probably missed other issues) a few comments to this line I noticed: pSlotList = (CK_SLOT_ID_PTR) malloc(ulSlotCount * sizeof(CK_SLOT_ID)); https://github.com/gamringer/php-pkcs11/blob/master/pkcs11.c#L300 1. The return value of malloc() is not checked. If the system runs out of memory (OOM) or for some other reason can't return a memory block this will return NULL and then lead to undefined behavior later. (unlikely on today's systems, but might happen) 2. emalloc should be used instead of malloc. For one it counts towards PHP's memory_limit, ten it also ensures the memory is freed if something weird happens by the end of the request, thus reduces risk of memory leaks and it fixes point 1. by terminating the request in an OOM situation. 3. ulSlotCount * sizeof(CK_SLOT_ID) could eventually overflow. Better use safe_emalloc(ulSlotCount, sizeof(CK_SLOT_ID), 0) this calculates ulSlotCount * sizeof(CK_SLOT_ID) + 0 in a overflow-safe way and errors out in case of a problem. johannes -- PECL development discussion Mailing List (https://pecl.php.net/) To unsubscribe, visit: https://www.php.net/unsub.php