243655 - pk11util crashes when using indexed array element

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

With pk11util, the following 2-line script crashes:

Newarray foo CK_ULONG 3
set foo[2] 999

The crash occurs because putOutput() clears the ArgStatic bit that was set 
by AddVariable() or varLookup(), and then argFreeData() tries to free the 
address of the array member, which crashes.  

The "fix" I am using is to not clear the argStatic bit in putOutput.

Also, I found that the array bounds check is off-by-1.  The 2-line script:

Newarray foo CK_ULONG 2
print foo[2]

should report index out of bounds, but does not.  I fixed this by changing
the > test to >= in  makeArrayTarget().  

Patch forthcoming shortly.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

This patch makes 3 changes:
1) fixes off-by-1 bounds check
2) removes line that clears the ArgStatic bit in putOutput().
3) simplifies error handling in main loop.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 148549 [details] [diff] [review]
patch v1

Bob, please review.

Comment 3

[Robert Relyea]

Comment on attachment 148549 [details] [diff] [review]
patch v1

Everyting looks good except the commented out ptr[i]->type &= ArgMask; line.

The better fix for the commented out line should be:

ptr[i]->type &= ~ArgOut;

It would also be acceptable to change it to:

ptr[i]-type &= ArgMask|ArgStatic;

but I think the best fix is just clearing the ArgOut flag.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

I made the change you suggested and checked it in.

/cvsroot/mozilla/security/nss/cmd/pk11util/pk11util.c,v  <--  pk11util.c
new revision: 1.6; previous revision: 1.5