Last Comment Bug 558160 - JAR_find and JAR_find_next use PORT_Strcmp for pattern matching
: JAR_find and JAR_find_next use PORT_Strcmp for pattern matching
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.12.4
: All All
: P2 normal (vote)
: 3.12.7
Assigned To: Nelson Bolyard (seldom reads bugmail)
:
:
Mentors:
http://mxr.mozilla.org/security/sourc...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-08 13:02 PDT by G. Richard Bellamy
Modified: 2010-04-24 17:56 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch file. (848 bytes, patch)
2010-04-08 13:03 PDT, G. Richard Bellamy
no flags Details | Diff | Splinter Review
alternative patch v1 (1.38 KB, patch)
2010-04-10 21:27 PDT, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Splinter Review

Description G. Richard Bellamy 2010-04-08 13:02:59 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US) AppleWebKit/533.2 (KHTML, like Gecko) Chrome/5.0.342.7 Safari/533.2
Build Identifier: NSS_3_12_4_RTM

JAR_Context indicates that pattern* supports regular expressions, it does not.

Because verify_global calls JAR_find with a pattern of "*", and JAR_find_next uses PORT_Strcmp to test the pattern against the current pathname, it never traverses the tree.

The patch does not make pattern tests regex-aware, instead, by passing in a NULL for the pattern, JAR_find_next fully traverses the tree.

See the patch in Additional Information.

Reproducible: Always

Steps to Reproduce:
1. signtool -k testcert -v --verbosity test.jar
Actual Results:  
Should output information about the MF, SF, DSA and RSA files found in the JAR.

Expected Results:  
It does not output information about MF, SF, DSA and RSA files found in JAR.

Index: security/nss/cmd/signtool/verify.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/verify.c,v
retrieving revision 1.5
diff -U 3 -r1.5 verify.c
--- security/nss/cmd/signtool/verify.c	16 Jul 2004 00:04:47 -0000	1.5
+++ security/nss/cmd/signtool/verify.c	8 Apr 2010 19:36:52 -0000
@@ -170,7 +170,7 @@
     int	          retval = 0;
     char	  buf [BUFSIZ];
 
-    ctx = JAR_find (jar, "*", jarTypePhy);
+    ctx = JAR_find (jar, NULL, jarTypePhy);
 
     while (JAR_find_next (ctx, &it) >= 0) {
 	if (!PORT_Strncmp (it->pathname, "META-INF", 8)) {
Comment 1 G. Richard Bellamy 2010-04-08 13:03:57 PDT
Created attachment 437930 [details] [diff] [review]
Patch file.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2010-04-09 09:55:06 PDT
I confirm the bug.  I confirm that all other callers of JAR_Find pass NULL
for the second argument.  This is the only caller of JAR_find that passes
a non-NULL value, so it is tempting to change it to pass NULL also, since
this will have the immediately desired effect.  I could give the provided
patch an r+ on that basis.  Maybe I should.

But clearly JAR_find and JAR_find_next are broken, don't work as intended.
This patch works around that problem, rather than fixing it.  The flaw in
those functions is due to the following code (as Richard noted above)
in jar.c:

451             if (ctx->pattern && *ctx->pattern) {
452                 if (PORT_Strcmp ((*it)->pathname, ctx->pattern))
453                     continue;

Since NSS has a pattern matching function, I'd rather try using it here,
than giving up on JAR_find* ever working as intended.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2010-04-09 10:09:00 PDT
Richard, 
Please try this patch to security/nss/lib/jar/jar.c as an alternative to your
patch to security/nss/cmd/signtool/verify.c

+#include "portreg.h"

-                if (PORT_Strcmp ((*it)->pathname, ctx->pattern))
+                if (PORT_RegExpSearch((*it)->pathname, ctx->pattern))
Comment 4 G. Richard Bellamy 2010-04-09 12:28:10 PDT
Actually, that was my first attempt, but it produced a compile error, even after I changed the manifest.mn to require util:

gcc -o Linux2.6_x86_64_glibc_PTH_64_DBG.OBJ/jar.o -c -g -fPIC -DLINUX1_2 -D_XOPEN_SOURCE -DLINUX2_1  -ansi -Wall -Werror-implicit-function-declaration -Wno-switch -pipe -DLINUX -Dlinux -D_POSIX_SOURCE -D_BSD_SOURCE -DHAVE_STRERROR -DXP_UNIX -DMOZILLA_CLIENT=1 -DDEBUG -UNDEBUG -DDEBUG_rbellamy -D_REENTRANT -DUSE_UTIL_DIRECTLY -DNSS_X86_OR_X64 -DNSS_X64 -I../../../../dist/Linux2.6_x86_64_glibc_PTH_64_DBG.OBJ/include -I../../../../dist/public/nss -I../../../../dist/private/nss -I../../../../dist/public/dbm -I../../../../dist/public/util  jar.c
jar.c: In function ‘JAR_find_next’:
jar.c:453: error: implicit declaration of function ‘PORT_RegExSearch’

I'm just not familiar enough with the build system yet to track this down, so I just did the simplest patch I could, given my limited time.

However, any guidance on how I would correct the above would be greatly appreciated, and I'll happily give it another shot.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2010-04-10 21:27:10 PDT
Created attachment 438310 [details] [diff] [review]
alternative patch v1

Try this patch.  (Beware: This patch has Windows line endings)
Comment 6 Robert Relyea 2010-04-22 16:54:17 PDT
Comment on attachment 438310 [details] [diff] [review]
alternative patch v1

r+

I also think exporting PORT_RegExpSearch is probably a good thing in itself as well.

bob
Comment 7 Nelson Bolyard (seldom reads bugmail) 2010-04-24 17:56:00 PDT
Bug 558160: JAR_find and JAR_find_next use PORT_Strcmp for pattern matching
r=rrelyea

Checking in jar/jar.c;        new revision: 1.6;  previous revision: 1.5
Checking in util/nssutil.def; 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.