Permalink
Choose a base ref
{{ refName }}
default
Choose a head ref
{{ refName }}
default
Comparing changes
Choose two branches to see what’s changed or to start a new pull request.
If you need to, you can also or
learn more about diff comparisons.
Open a pull request
Create a new pull request by comparing changes across two branches. If you need to, you can also .
Learn more about diff comparisons here.
base repository: sailfishos-mirror/nss
base: f945d56b45e026bcf0a307419758e68c85ad3b47
Could not load branches
Nothing to show
Could not load tags
Nothing to show
{{ refName }}
default
...
head repository: sailfishos-mirror/nss
compare: 035673e21d0d3da243ec34c823ff47debb3772e5
Could not load branches
Nothing to show
Could not load tags
Nothing to show
{{ refName }}
default
- 3 commits
- 41 files changed
- 1 contributor
Commits on Mar 11, 2021
-
Bug 1697303 NSS needs to update it's csp clearing to FIPS 180-3 stand…
…ards. FIPS 180-3 updated the standard for clearing sensitive key material in FIPS modules. I've done a complete review of the portions of NSS affected by the FIPS requirements and identified all the areas where we need to update. The report is available here: https://docs.google.com/document/d/1v9kedUiwVYYIUagyT_vQdtrktjGUrA3SFsVP-LA6vOw/edit?usp=sharing This patch does the following: - Clears the stack in gcm and ecc to deal with large stack leakages. This only happens in FIPS enabled case. The size of the stack is based on the size of the leakage, with some extra to make sure we reach down into that area. Most of the leakage happens in either auto generated code or machine dependent acceleration code. - Clears hash related data that wasn't cleared previously - Clears public key exponents that wasn't cleared previously. - Clears components that should have been cleared previously but wasn't. Usually clearing takes one of the following forms: PORT_Free(x) -> PORT_Free(x, size). This means we need to know what the size is supposed to be. In some cases we need to add code to preserve the size. PORT_Free(x.data) -> SECITEM_ZfreeItem(&x, PR_FALSE). In this case x is a SECITEM, which carries the length. PR_FALSE means clear and free the data in the item, not the item itself. The code should have had SECITEM_FreeItem before anyway. SECIEM_FreeItem(item, bool) -> SECITEM_ZfreeItem(item, bool). Simply change the normal SECITEM free call to the one that clears the item. PR_ArenaFree(arena, PR_FALSE) -> PR_ArenaFree(arena, PR_TRUE). The bool here means whether or not to clear as well as free the data in the arena. PORT_Memset(value, 0, size). This the obvious clear operation. It happens if the variable is a stack variable, or if the memory isn't cleared with one of the three clearing functions above. In addition this patch fixes the following: - moves the determination if whether or not a slot is in FIPS mode by slotID to a macro. NSS allows user defined slots to be opened. If you open a user defined slot using the FIPS slot, the resulting slots will also be FIPS slots. In areas where the semantics change based on the slot, these slots should have the FIPS semantics. Directly checking if the slot is the FIPS slot now only happens when we really mean the main FIPS slot and not just any FIPS slot. - In handling the clearing of PSS and OAEP, I identified an issue. These functions where holding a pointer to the pMechanismParams in their C_XXXXInit calls for later use in the C_XXXXUpdate/C_XXXXFinal/C_XXXX calls. The problem is applications are allowed to free their pMechanismParams once C_XXXXInit is complete. We need to make a copy of the params to use them. Differential Revision: https://phabricator.services.mozilla.com/D108223 --HG-- extra : rebase_source : 7ac6f9d40bffbaf1a45e9e0996893afe618d4fc5
Commits on Apr 8, 2021
-
Bug 1703936 New coverity/cpp scanner errors.
Redhat has run our scanners on the full NSS tree and identifed 123 errors our security team determined where 'critical'. I've reviewed the errors and identified a much smaller subset of errors that are either real, or confusing enough to warrent suppression comments. Many errors are in cmd and gtest. I've skip those commands red hat does not ship in this report, and I've skipped the issues in gtest. Also, There's a large number of leaked_storage errors because evidently coverity gets confused when you have a pointer in a local variable and you pass that pointer off to a global or a function variable. I've skipped most of those as well. changes: crlutil.c: add missing arena free in error path. #def4 secutil.c: (Not coverity found) make sure we don't overflow our buffer in badly encoded ECC oids. modultil.c: Fix incorrect free operation in pk11install case. #def6 signtool/javascript.c: free old archiveDir after use in PR_smprintf #def8 don't double free curitem (curitem is almost certainly NULL at this point, so the current code is a noop). #def9, #def10 signtool/list.c: remove unused ugly_list variable (which is leaked) #def12 signtool/util.c: don't leak 'dir' in error path #def13 #def14 sigver/pk7print.c: coverity: use static pk rather than an allocated and leaked pointer. #def15 add code for EC disbled DSA code that isn't actually working (PQG params). symkeyutil.c: free name (depends on PORT_Free null check). #def16 pkix_pl_nameconstraints.c: Fix coverity double free warning. PKIX_ERROR_RECEIVED is almost certainly false in this case, but by setting arena to NULL we make sure it's not used or freed again. #def99 pkix_pl_string.c: Fix varargs leak in the error path. #def109 pk11parse.c: secmod_doDescCopy can reallocate our newSpec, but the pointer we are passed is an offset from newSpec. Pass in both pointers and return our newly allocated spec and length in that case.#def113 cmsutil.c: suppress cppcheck warnings do to cmsutil use of unions to cast pointers. #def113-117 pkcs11.c: support coverity incorrect use_after_free warning. #def118 scanner errors: Error: USE_AFTER_FREE (CWE-416): [#def4] nss-3.60.1/nss/cmd/crlutil/crlutil.c:389: freed_arg: "PORT_FreeArena_Util" frees "modArena". nss-3.60.1/nss/cmd/crlutil/crlutil.c:455: double_free: Calling "PORT_FreeArena_Util" frees pointer "modArena" which has already been freed. 453| } 454| if (modArena && (!modCrl || modCrl->arena != modArena)) { 455|-> PORT_FreeArena(modArena, PR_FALSE); 456| } 457| if (modCrl) Error: BAD_FREE (CWE-590): [#def6] nss-3.60.1/nss/cmd/modutil/install-ds.c:1046: address_free: "PR_Free" frees address of "_this->forwardCompatible". 1044| Pk11Install_PlatformName_delete(&_this->forwardCompatible[i]); 1045| } 1046|-> PR_Free(&_this->forwardCompatible); 1047| _this->numForwardCompatible = 0; 1048| } Error: USE_AFTER_FREE (CWE-416): [#def8] nss-3.60.1/nss/cmd/signtool/javascript.c:1346: freed_arg: "PR_Free" frees "archiveDir". nss-3.60.1/nss/cmd/signtool/javascript.c:1347: pass_freed_arg: Passing freed pointer "archiveDir" as an argument to "PR_smprintf". 1345| warningCount++; 1346| PR_Free(archiveDir); 1347|-> archiveDir = PR_smprintf("%s.arc", archiveDir); 1348| } else { 1349| PL_strcpy(archiveDir + strlen(archiveDir) - 4, ".arc"); Error: USE_AFTER_FREE (CWE-416): [#def9] nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail". nss-3.60.1/nss/cmd/signtool/javascript.c:1654: double_free: Calling "PR_Free" frees pointer "curitem" which has already been freed. 1652| } 1653| if (curitem) { 1654|-> PR_Free(curitem); 1655| } 1656| if (basedir) { Error: USE_AFTER_FREE (CWE-416): [#def10] nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityListTail" = "entityItem". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1477: alias: Assigning: "entityList" = "entityListTail". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1623: alias: Assigning: "curitem" = "entityList". Now both point to the same storage. nss-3.60.1/nss/cmd/signtool/javascript.c:1651: freed_arg: "PR_Free" frees "entityListTail". nss-3.60.1/nss/cmd/signtool/javascript.c:1654: pass_freed_arg: Passing freed pointer "curitem" as an argument to "PR_Free". 1652| } 1653| if (curitem) { 1654|-> PR_Free(curitem); 1655| } 1656| if (basedir) { Error: RESOURCE_LEAK (CWE-772): [#def12] nss-3.60.1/nss/cmd/signtool/list.c:36: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util". nss-3.60.1/nss/cmd/signtool/list.c:36: var_assign: Assigning: "ugly_list" = storage returned from "PORT_ZAlloc_Util(16UL)". nss-3.60.1/nss/cmd/signtool/list.c:137: leaked_storage: Variable "ugly_list" going out of scope leaks the storage it points to. 135| 136| if (failed) { 137|-> return -1; 138| } 139| return 0; Error: RESOURCE_LEAK (CWE-772): [#def13] nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir". nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)". nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument. nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir". nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)". nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "entry" going out of scope leaks the storage it points to. nss-3.60.1/nss/cmd/signtool/util.c:142: leaked_storage: Variable "dir" going out of scope leaks the storage it points to. 140| if (snprintf(filename, sizeof(filename), "%s/%s", path, entry->name) >= sizeof(filename)) { 141| errorCount++; 142|-> return -1; 143| } 144| if (rm_dash_r(filename)) Error: RESOURCE_LEAK (CWE-772): [#def14] nss-3.60.1/nss/cmd/signtool/util.c:131: alloc_fn: Storage is returned from allocation function "PR_OpenDir". nss-3.60.1/nss/cmd/signtool/util.c:131: var_assign: Assigning: "dir" = storage returned from "PR_OpenDir(path)". nss-3.60.1/nss/cmd/signtool/util.c:139: identity_transfer: Passing "dir" as argument 1 to function "PR_ReadDir", which returns an offset off that argument. nss-3.60.1/nss/cmd/signtool/util.c:139: noescape: Resource "dir" is not freed or pointed-to in "PR_ReadDir". nss-3.60.1/nss/cmd/signtool/util.c:139: var_assign: Assigning: "entry" = storage returned from "PR_ReadDir(dir, PR_SKIP_BOTH)". nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "entry" going out of scope leaks the storage it points to. nss-3.60.1/nss/cmd/signtool/util.c:145: leaked_storage: Variable "dir" going out of scope leaks the storage it points to. 143| } 144| if (rm_dash_r(filename)) 145|-> return -1; 146| } 147| Error: RESOURCE_LEAK (CWE-772): [#def15] nss-3.60.1/nss/cmd/signver/pk7print.c:325: alloc_fn: Storage is returned from allocation function "PORT_ZAlloc_Util". nss-3.60.1/nss/cmd/signver/pk7print.c:325: var_assign: Assigning: "pk" = storage returned from "PORT_ZAlloc_Util(328UL)". nss-3.60.1/nss/cmd/signver/pk7print.c:351: leaked_storage: Variable "pk" going out of scope leaks the storage it points to. 349| default: 350| fprintf(out, "%s=bad SPKI algorithm type\n", msg); 351|-> return 0; 352| } 353| Error: RESOURCE_LEAK (CWE-772): [#def16] nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: alloc_fn: Storage is returned from allocation function "PK11_GetSymKeyNickname". nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:289: var_assign: Assigning: "name" = storage returned from "PK11_GetSymKeyNickname(symKey)". nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:298: noescape: Resource "name ? name : " "" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.] nss-3.60.1/nss/cmd/symkeyutil/symkeyutil.c:306: leaked_storage: Variable "name" going out of scope leaks the storage it points to. 304| } 305| printf("\n"); 306|-> } 307| 308| SECStatus Error: USE_AFTER_FREE (CWE-416): [#def99] nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:835: freed_arg: "PORT_FreeArena_Util" frees "arena". nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/pki/pkix_pl_nameconstraints.c:854: double_free: Calling "PORT_FreeArena_Util" frees pointer "arena" which has already been freed. 852| PKIX_CERTNAMECONSTRAINTS_DEBUG 853| ("\t\tCalling PORT_FreeArena).\n"); 854|-> PORT_FreeArena(arena, PR_FALSE); 855| } 856| } Error: VARARGS (CWE-237): [#def109] nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:428: va_init: Initializing va_list "args". nss-3.60.1/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_string.c:534: missing_va_end: "va_end" was not called for "args". 532| } 533| 534|-> PKIX_RETURN(STRING); 535| } 536| Error: USE_AFTER_FREE (CWE-416): [#def112] nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1099: alias: Assigning: "newSpecPtr" = "newSpec". Now both point to the same storage. nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1156: freed_arg: "secmod_doDescCopy" frees "newSpecPtr". nss-3.60.1/nss/lib/pk11wrap/pk11pars.c:1211: use_after_free: Using freed pointer "newSpec". 1209| /* no target found, return the newSpec */ 1210| if (target == NULL) { 1211|-> return newSpec; 1212| } 1213| Error: CPPCHECK_WARNING (CWE-562): [#def113] nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'digestedData' that will be invalid when returning. 307| } 308| } 309|-> return cinfo; 310| } 311| Error: CPPCHECK_WARNING (CWE-562): [#def114] nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'encryptedData' that will be invalid when returning. 307| } 308| } 309|-> return cinfo; 310| } 311| Error: CPPCHECK_WARNING (CWE-562): [#def115] nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'envelopedData' that will be invalid when returning. 307| } 308| } 309|-> return cinfo; 310| } 311| Error: CPPCHECK_WARNING (CWE-562): [#def116] nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'genericData' that will be invalid when returning. 307| } 308| } 309|-> return cinfo; 310| } 311| Error: CPPCHECK_WARNING (CWE-562): [#def117] nss-3.60.1/nss/lib/smime/cmsutil.c:309: error[returnDanglingLifetime]: Returning pointer to local variable 'signedData' that will be invalid when returning. 307| } 308| } 309|-> return cinfo; 310| } 311| Error: USE_AFTER_FREE (CWE-416): [#def118] nss-3.60.1/nss/lib/softoken/pkcs11.c:2671: freed_arg: "PORT_Realloc_Util" frees "oldNscSlotList". nss-3.60.1/nss/lib/softoken/pkcs11.c:2674: use_after_free: Using freed pointer "oldNscSlotList". 2672| nscSlotListSize[index] * sizeof(CK_SLOT_ID)); 2673| if (nscSlotList[index] == NULL) { 2674|-> nscSlotList[index] = oldNscSlotList; 2675| nscSlotListSize[index] = oldNscSlotListSize; 2676| return CKR_HOST_MEMORY; Some of these are false positives, but they are reasonable issues for the scanners to flag, so scanner suppression comments, along with human reviewer comments will be added. Differential Revision: https://phabricator.services.mozilla.com/D111339 --HG-- extra : rebase_source : 45ad7590affb7fdb8a0e182e29c6c88f909ff260
Commits on Apr 29, 2021
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff f945d56b45e026bcf0a307419758e68c85ad3b47...035673e21d0d3da243ec34c823ff47debb3772e5