The default bug view has changed. See this FAQ.

remove unneeded assert from sec_PKCS7EncryptLength

RESOLVED FIXED in 3.12.1

Status

NSS
Libraries
P2
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Dolske, Assigned: Dolske)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

823 bytes, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
http://mxr.mozilla.org/seamonkey/source/security/nss/lib/pkcs7/p7local.c#588

This function has the following snippet of code:

582     /*
583      * If we just did our very last block, "remove" the padding by
584      * adjusting the output length.
585      */
586     if (final && (padsize != 0)) {
587         unsigned int padlen = *(output + ofraglen - 1);
588         PORT_Assert (padlen > 0 && padlen <= padsize);
589         if (padlen == 0 || padlen > padsize) {
590             PORT_SetError (SEC_ERROR_BAD_DATA);
591             return SECFailure;
592         }
593         output_len -= padlen;
594     }

Nothing wrong here, except that I've got a unit test that ends up hitting the assert when trying to decrypt some data with the wrong password... The final padding block is thus garbage (as one would expect).

The unfortunate effect is that when running tests against a debug build, the assert is fatal (process crashes). The fix would be to just remove like 588, as the line below it checks the same thing and returns an error.
yes, I agree.  Now, if someone were to submit a patch that I can review... :)
(Assignee)

Comment 2

9 years ago
Created attachment 323017 [details] [diff] [review]
Patch v.1
Assignee: nobody → dolske
Status: NEW → ASSIGNED
Attachment #323017 - Flags: review?(nelson)
Comment on attachment 323017 [details] [diff] [review]
Patch v.1

Thanks.  r=me

Checking in p7local.c;
new revision: 1.13; previous revision: 1.12
Attachment #323017 - Flags: review?(nelson) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.12.1
Version: unspecified → trunk
(Assignee)

Updated

9 years ago
Blocks: 435120
You need to log in before you can comment on or make changes to this bug.