Last Comment Bug 436428 - remove unneeded assert from sec_PKCS7EncryptLength
: remove unneeded assert from sec_PKCS7EncryptLength
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: trunk
: All All
: P2 minor (vote)
: 3.12.1
Assigned To: Justin Dolske [:Dolske]
:
Mentors:
Depends on:
Blocks: 435120
  Show dependency treegraph
 
Reported: 2008-05-29 17:50 PDT by Justin Dolske [:Dolske]
Modified: 2008-06-26 16:11 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v.1 (823 bytes, patch)
2008-05-29 18:42 PDT, Justin Dolske [:Dolske]
nelson: review+
Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2008-05-29 17:50:39 PDT
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.
Comment 1 Nelson Bolyard (seldom reads bugmail) 2008-05-29 18:06:25 PDT
yes, I agree.  Now, if someone were to submit a patch that I can review... :)
Comment 2 Justin Dolske [:Dolske] 2008-05-29 18:42:14 PDT
Created attachment 323017 [details] [diff] [review]
Patch v.1
Comment 3 Nelson Bolyard (seldom reads bugmail) 2008-05-29 20:40:32 PDT
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

Note You need to log in before you can comment on or make changes to this bug.