369467 - signtool crashes when creating a JAR

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Wolfgang Rosenauer [:wolfiR]]

signtool crashes when creating a JAR with SIG11.

NSPR 4.6.5
NSS 3.11.5

I'm trying to execute
signtool -k "Cert-ID" -Z "testy.jar" content/

at some point (it seems to be a certain xul file in my case) it crashes and I get the following backtrace:

0x00002b7645b7bfbe in PL_strncasecmp (a=0x80006514a36f <Address 0x80006514a36f out of bounds>, b=0x4b04b0 ".arc", max=4) at strccmp.c:104
104         while( max && (uc[*ua] == uc[*ub]) && ('\0' != *a) )
(gdb) bt
#0  0x00002b7645b7bfbe in PL_strncasecmp (a=0x80006514a36f <Address 0x80006514a36f out of bounds>, b=0x4b04b0 ".arc", max=4) at strccmp.c:104
#1  0x00002b7645b7c6d5 in PL_strcaserstr (big=0x7fff6514a370 "tvp", little=0x4b04b0 ".arc") at strcstr.c:73
#2  0x000000000040ec6c in manifesto_fn ()
#3  0x0000000000410760 in foreach ()
#4  0x00000000004106ee in foreach ()
#5  0x000000000040e455 in manifesto ()
#6  0x000000000040f487 in SignArchive ()
#7  0x0000000000409fe8 in main ()
#8  0x00002b7646629ae4 in __libc_start_main () from /lib64/libc.so.6
#9  0x0000000000408789 in _start ()

Comment 1

[Wolfgang Rosenauer [:wolfiR]]

It seems that PL_strcaserstr() has to be called with the bigger string as first argument. That's not the case for me since the first string (a directory name) has 3 characters and the second one (.arc) has four characters, which results in

p = &big[ -1 ];
which is obviously wrong and results in p being a pointer to somewhere but not in the space of the string.

Comment 2

[Wolfgang Rosenauer [:wolfiR]]

Attachment: patch

If arg1 really has to be bigger than arg2 this patch should solve the crash.
If the function should handle it otherwise it would need some more changes.

Comment 3

[Wolfgang Rosenauer [:wolfiR]]

finally it's an NSPR bug

Comment 4

[Wan-Teh Chang]

Attachment: patch v2

Wolfgang, thank you for tracking down the bug and proposing a patch.
I improved your patch:
1. I save the value of PL_strlen(big) in a local variable so that
we don't compute it twice.
2. I remove the redundant test "if( p < big ) return (char *)0;"

Please review and test this patch.  Thanks!

Comment 5

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 255026 [details] [diff] [review]
patch v2

Thanks Wan-Teh, that looks better and works.

Comment 6

[Wan-Teh Chang]

I wrote a short test program to demonstrate this bug.
This is a 64-bit problem.  The test program foo.c is:

$ cat foo.c
#include <stdio.h>

int main()
{
    unsigned i, j;
    char big[20];
    char *p;

    i = 2;
    j = 5;
    p = &big[i-j];

    printf("p=%p big=%p\n", p, big);
    if (p < big) {
        printf("p smaller\n");
    } else {
        printf("p bigger\n");
    }
    return 0;
}

If I compile and run it on 32-bit Linux, the output is correct:
$ gcc foo.c
$ ./a.out
p=0xbfe7ed7d big=0xbfe7ed80
p smaller

If I compile and run it on 64-bit Linux, the output is wrong:
$ gcc foo.c
$ ./a.out
p=0x80bffff97d big=0x7fbffff980
p bigger

It seems that the problem depends on whether i-j is treated
as a 32-bit or 64-bit value.

I checked in the patch on the NSPR trunk (NSPR 4.7) and the
NSPRPUB_PRE_4_2_CLIENT_BRANCH (Mozilla 1.9 Alpha 2).

Checking in strcstr.c;
/cvsroot/mozilla/nsprpub/lib/libc/src/strcstr.c,v  <--  strcstr.c
new revision: 3.7; previous revision: 3.6
done

Checking in strcstr.c;
/cvsroot/mozilla/nsprpub/lib/libc/src/strcstr.c,v  <--  strcstr.c
new revision: 3.4.4.3; previous revision: 3.4.4.2
done

Wolfgang, let me know if you need this bug fixed in an NSPR 4.6.x
patch release.  Can you work around this bug?

Comment 7

[Wolfgang Rosenauer [:wolfiR]]

Interesting compiler behaviour. On x86-64 the array index seems to be unsigned (and therefore "underflowed" resulting in an index like big[4294967293] while on x86 the index can be negative. Anyway ;-)

I don't need this bug fixed in 4.6.x. Thanks.