User Agent: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20100101 Firefox/14.0.1 Build ID: 20120713134347 Steps to reproduce: I was grep for something else in libre office and found this, which is part of firefox Expected results: At line 1476 of source/security/nss/cmd/certcgi/certcgi.c, a assignment seems really strange to me : low_digit = *string = 'A'; According to surrounding code, I think it should be : low_digit = *string - 'A'; I'm not sure at all because I don't really understand what that code do (high_digit and low_digit seem to be unused).
Assignee: nobody → nobody
Component: Untriaged → Libraries
Product: Firefox → NSS
Version: 14 Branch → unspecified
Yes, that's it.
wtc: The line still seems present in NSS, should it be - instead of = as Christophe suggested?
Hi. Those two lines should read high_digit = *string - 'A' + 10; low_digit = *string - 'A' + 10; We need to subtract 'A' from the hex digit character, and then add 10 (the value of the hex digit A).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 745609 [details] [diff] [review] Proposed Patch v1 This patch changes the lines to match those in Comment 4.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Attachment #745609 - Flags: review?(wtc)
Comment on attachment 745609 [details] [diff] [review] Proposed Patch v1 r=wtc. https://hg.mozilla.org/projects/nss/rev/0babcec0543a
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → 3.15
wtc: Sorry for not noticing this earlier, but I get compiler warnings that high_digit and low_digit in string_to_binary() are not used... Should they be removed?
Cykesiopka: We should use high_digit and low_digit in string_to_binary(). You can search for "low_digit" in the file to figure out how high_digit and low_digit should be combined into a byte and stored in rv->data[rv->len]. I also found that the caller of string_to_binary() only frees the SECItem |temp|, but not the |temp->data| buffer, so there is also a memory leak. We need to be careful there because in the other branch, we have temp = (SECItem *) PORT_ZAlloc(sizeof(SECItem)); ... temp->data = (unsigned char *)name; so in that branch we should only free |temp|.
Hmm, I would assume the high_digit and low_digit should be combined similar to this: rv->data[rv->len] = (hi_digit << 4) | low_digit; Unfortunately I can't figure out where to free |temp->data|. |genName->name.OthName.name.data| is set to |temp->data|, but I don't know where that is used at all... Of course, I might just be having some sort of fundamental misunderstanding, apologies...
You need to log in before you can comment on or make changes to this bug.