Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xmlsec-nss: CERT_FindCertBySubjectKeyID does not work in all cases #4

Open
lsh123 opened this issue Jan 28, 2016 · 0 comments
Open

xmlsec-nss: CERT_FindCertBySubjectKeyID does not work in all cases #4

lsh123 opened this issue Jan 28, 2016 · 0 comments
Labels
blocked bug

Comments

@lsh123
Copy link
Owner

lsh123 commented Jan 28, 2016

NSS bug: http://bugzilla.mozilla.org/show_bug.cgi?id=211051

Added code to search for certs in the imported list, still would love to have proper NSS support.

Migrated from: https://bugzilla.gnome.org/show_bug.cgi?id=118632
@lsh123 lsh123 changed the title xmlsec-nss: CERT_FindCertBySubjectKeyID does not work in all cases (edit) xmlsec-nss: CERT_FindCertBySubjectKeyID does not work in all cases Dec 8, 2017
vmiklos added a commit to vmiklos/xmlsec that referenced this issue Sep 7, 2018
@vmiklos
nss: fix undefined behavior due to too large shift
e470f35 
When building with clang -fsanitize=undefined, ubsan says:

x509.c:1750:46: runtime error: shift exponent 64 is too large for 64-bit type 'PRUint64' (aka 'unsigned long')
    #0 0x444d45 in xmlSecNssASN1IntegerWrite src/nss/x509.c:1750:46
    lsh123#1 0x4443ec in xmlSecNssX509IssuerSerialNodeWrite src/nss/x509.c:1259:11
    lsh123#2 0x4403ba in xmlSecNssKeyDataX509XmlWrite src/nss/x509.c:769:19
    lsh123#3 0x45962a in xmlSecKeyInfoNodeWrite src/keyinfo.c:180:19
    lsh123#4 0x480149 in xmlSecDSigCtxProcessKeyInfoNode src/xmldsig.c:807:15
    lsh123#5 0x47c774 in xmlSecDSigCtxProcessSignatureNode src/xmldsig.c:506:11
    lsh123#6 0x47bfb2 in xmlSecDSigCtxSign src/xmldsig.c:289:11

And indeed shifting a 64bit value by 64 bits happens in practice there
as num->len is 9. At the same time (at least in case of the test) in all
3 cases the value that would be shifted is 0.

Avoid undefined behavior by simply not shifting if the value is 0
anyway.

Testcase: make check-crypto-nss XMLSEC_TEST_NAME="aleksey-xmldsig-01/x509data-sn-test"
@vmiklos vmiklos mentioned this issue Sep 7, 2018
Merged
lsh123 pushed a commit that referenced this issue Sep 8, 2018
@vmiklos @lsh123
nss: fix undefined behavior due to too large shift (#221)
10c9708 
When building with clang -fsanitize=undefined, ubsan says:

x509.c:1750:46: runtime error: shift exponent 64 is too large for 64-bit type 'PRUint64' (aka 'unsigned long')
    #0 0x444d45 in xmlSecNssASN1IntegerWrite src/nss/x509.c:1750:46
    #1 0x4443ec in xmlSecNssX509IssuerSerialNodeWrite src/nss/x509.c:1259:11
    #2 0x4403ba in xmlSecNssKeyDataX509XmlWrite src/nss/x509.c:769:19
    #3 0x45962a in xmlSecKeyInfoNodeWrite src/keyinfo.c:180:19
    #4 0x480149 in xmlSecDSigCtxProcessKeyInfoNode src/xmldsig.c:807:15
    #5 0x47c774 in xmlSecDSigCtxProcessSignatureNode src/xmldsig.c:506:11
    #6 0x47bfb2 in xmlSecDSigCtxSign src/xmldsig.c:289:11

And indeed shifting a 64bit value by 64 bits happens in practice there
as num->len is 9. At the same time (at least in case of the test) in all
3 cases the value that would be shifted is 0.

Avoid undefined behavior by simply not shifting if the value is 0
anyway.

Testcase: make check-crypto-nss XMLSEC_TEST_NAME="aleksey-xmldsig-01/x509data-sn-test"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
blocked bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lsh123