You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@santuario.apache.org by "Paulo Zanoni (JIRA)" <ji...@apache.org> on 2011/05/17 18:15:47 UTC
[jira] [Updated] (SANTUARIO-272) Memory bug inside
XENCCipherImpl::deSerialise
[ https://issues.apache.org/jira/browse/SANTUARIO-272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paulo Zanoni updated SANTUARIO-272:
-----------------------------------
Description:
Hello again
Valgrind detects an error at XENCCipherImpl::deSerialise. It says we're reading an unitialized value.
I couldn't figure out exactly what's going on with that code, but it seems the call to "att->getNodeName()[5]" is triggering the Valgrind message. I also added some "std::cout" to that code and checked. One example is the following:
In my case, one of the calls has:
- att->getNodeName() is an XMLString containing "a"
- DSIGConstants::s_unicodeStrXmlns is an XMLString containing "xmlns"
Now look at this statement:
if (strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns) || (XMLString::compareNString(
att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) && att->getNodeName()[5] == chColon)) {
So what happens is this:
- The first part of the "if" statement is:
"strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns)"
In this case, the value returned is "0", which is ok.
- Next, the function will try to do:
"XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5)"
In this case, the value returned is 1, because one string is "bigger" than the other. See the description at /usr/include/xercesc/util/XMLString.hpp
- Since the first statement of the "&&" operation returned "1", the code will proceed to evaluate the last part of the "if" statement:
"att->getNodeName()[5] == chColon"
But "att->getNodeName()" is an XMLString containing "a". You can't access it at index "5". This is why valgrind gives us the error message.
I really didn't try to understand exactly what is going with that code, but my guess is that what you really wanted to do is to use:
"(XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) == 0)"
instead of the statement without the "== 0". I tried to patch it locally and test, and it _seems_ to be working, but I can't guarantee anything.
I'll attach a code example that reproduces the bug. Run it with valgrind and you'll see the messages. It will look like this:
paulo@foobar:~/teste/xerces/deserializer$ valgrind ./reproducer
==7410== Memcheck, a memory error detector
==7410== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==7410== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==7410== Command: ./reproducer
==7410==
==7410== Conditional jump or move depends on uninitialised value(s)
==7410== at 0x444A517: XENCCipherImpl::deSerialise(safeBuffer&, xercesc_3_1::DOMNode*) (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x444C0CE: XENCCipherImpl::decryptElementDetached() (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x444877F: XENCCipherImpl::decryptElement() (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x44499E6: XENCCipherImpl::decryptElement(xercesc_3_1::DOMElement*) (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x804A389: decryptXmlFile(xercesc_3_1::DOMDocument*, XSECCryptoKey*) (reproducer.cpp:93)
==7410== by 0x804A560: main (reproducer.cpp:119)
==7410==
==7410==
==7410== HEAP SUMMARY:
==7410== in use at exit: 44 bytes in 1 blocks
==7410== total heap usage: 7,284 allocs, 7,283 frees, 1,579,829 bytes allocated
==7410==
==7410== LEAK SUMMARY:
==7410== definitely lost: 0 bytes in 0 blocks
==7410== indirectly lost: 0 bytes in 0 blocks
==7410== possibly lost: 0 bytes in 0 blocks
==7410== still reachable: 44 bytes in 1 blocks
==7410== suppressed: 0 bytes in 0 blocks
==7410== Rerun with --leak-check=full to see details of leaked memory
==7410==
==7410== For counts of detected and suppressed errors, rerun with: -v
==7410== Use --track-origins=yes to see where uninitialised values come from
==7410== ERROR SUMMARY: 5 errors from 1 contexts (suppressed: 41 from 8)
was:
Hello again
Valgrind detects an error at XENCCipherImpl::doSerialise. It says we're reading an unitialized value.
I couldn't figure out exactly what's going on with that code, but it seems the call to "att->getNodeName()[5]" is triggering the Valgrind message. I also added some "std::cout" to that code and checked. One example is the following:
In my case, one of the calls has:
- att->getNodeName() is an XMLString containing "a"
- DSIGConstants::s_unicodeStrXmlns is an XMLString containing "xmlns"
Now look at this statement:
if (strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns) || (XMLString::compareNString(
att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) && att->getNodeName()[5] == chColon)) {
So what happens is this:
- The first part of the "if" statement is:
"strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns)"
In this case, the value returned is "0", which is ok.
- Next, the function will try to do:
"XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5)"
In this case, the value returned is 1, because one string is "bigger" than the other. See the description at /usr/include/xercesc/util/XMLString.hpp
- Since the first statement of the "&&" operation returned "1", the code will proceed to evaluate the last part of the "if" statement:
"att->getNodeName()[5] == chColon"
But "att->getNodeName()" is an XMLString containing "a". You can't access it at index "5". This is why valgrind gives us the error message.
I really didn't try to understand exactly what is going with that code, but my guess is that what you really wanted to do is to use:
"(XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) == 0)"
instead of the statement without the "== 0". I tried to patch it locally and test, and it _seems_ to be working, but I can't guarantee anything.
I'll attach a code example that reproduces the bug. Run it with valgrind and you'll see the messages. It will look like this:
paulo@foobar:~/teste/xerces/deserializer$ valgrind ./reproducer
==7410== Memcheck, a memory error detector
==7410== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==7410== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==7410== Command: ./reproducer
==7410==
==7410== Conditional jump or move depends on uninitialised value(s)
==7410== at 0x444A517: XENCCipherImpl::deSerialise(safeBuffer&, xercesc_3_1::DOMNode*) (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x444C0CE: XENCCipherImpl::decryptElementDetached() (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x444877F: XENCCipherImpl::decryptElement() (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x44499E6: XENCCipherImpl::decryptElement(xercesc_3_1::DOMElement*) (in /usr/lib/libxml-security-c.so.16.0.1)
==7410== by 0x804A389: decryptXmlFile(xercesc_3_1::DOMDocument*, XSECCryptoKey*) (reproducer.cpp:93)
==7410== by 0x804A560: main (reproducer.cpp:119)
==7410==
==7410==
==7410== HEAP SUMMARY:
==7410== in use at exit: 44 bytes in 1 blocks
==7410== total heap usage: 7,284 allocs, 7,283 frees, 1,579,829 bytes allocated
==7410==
==7410== LEAK SUMMARY:
==7410== definitely lost: 0 bytes in 0 blocks
==7410== indirectly lost: 0 bytes in 0 blocks
==7410== possibly lost: 0 bytes in 0 blocks
==7410== still reachable: 44 bytes in 1 blocks
==7410== suppressed: 0 bytes in 0 blocks
==7410== Rerun with --leak-check=full to see details of leaked memory
==7410==
==7410== For counts of detected and suppressed errors, rerun with: -v
==7410== Use --track-origins=yes to see where uninitialised values come from
==7410== ERROR SUMMARY: 5 errors from 1 contexts (suppressed: 41 from 8)
Summary: Memory bug inside XENCCipherImpl::deSerialise (was: Memory bug inside XENCCipherImpl::doSerialise)
> Memory bug inside XENCCipherImpl::deSerialise
> ---------------------------------------------
>
> Key: SANTUARIO-272
> URL: https://issues.apache.org/jira/browse/SANTUARIO-272
> Project: Santuario
> Issue Type: Bug
> Components: C++
> Affects Versions: C++ 1.6.0
> Environment: Ubuntu 11.04 i386
> Reporter: Paulo Zanoni
> Assignee: Scott Cantor
> Labels: bug, patch
>
> Hello again
> Valgrind detects an error at XENCCipherImpl::deSerialise. It says we're reading an unitialized value.
> I couldn't figure out exactly what's going on with that code, but it seems the call to "att->getNodeName()[5]" is triggering the Valgrind message. I also added some "std::cout" to that code and checked. One example is the following:
> In my case, one of the calls has:
> - att->getNodeName() is an XMLString containing "a"
> - DSIGConstants::s_unicodeStrXmlns is an XMLString containing "xmlns"
> Now look at this statement:
> if (strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns) || (XMLString::compareNString(
> att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) && att->getNodeName()[5] == chColon)) {
> So what happens is this:
> - The first part of the "if" statement is:
> "strEquals(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns)"
> In this case, the value returned is "0", which is ok.
> - Next, the function will try to do:
> "XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5)"
> In this case, the value returned is 1, because one string is "bigger" than the other. See the description at /usr/include/xercesc/util/XMLString.hpp
> - Since the first statement of the "&&" operation returned "1", the code will proceed to evaluate the last part of the "if" statement:
> "att->getNodeName()[5] == chColon"
> But "att->getNodeName()" is an XMLString containing "a". You can't access it at index "5". This is why valgrind gives us the error message.
> I really didn't try to understand exactly what is going with that code, but my guess is that what you really wanted to do is to use:
> "(XMLString::compareNString(att->getNodeName(), DSIGConstants::s_unicodeStrXmlns, 5) == 0)"
> instead of the statement without the "== 0". I tried to patch it locally and test, and it _seems_ to be working, but I can't guarantee anything.
> I'll attach a code example that reproduces the bug. Run it with valgrind and you'll see the messages. It will look like this:
> paulo@foobar:~/teste/xerces/deserializer$ valgrind ./reproducer
> ==7410== Memcheck, a memory error detector
> ==7410== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
> ==7410== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
> ==7410== Command: ./reproducer
> ==7410==
> ==7410== Conditional jump or move depends on uninitialised value(s)
> ==7410== at 0x444A517: XENCCipherImpl::deSerialise(safeBuffer&, xercesc_3_1::DOMNode*) (in /usr/lib/libxml-security-c.so.16.0.1)
> ==7410== by 0x444C0CE: XENCCipherImpl::decryptElementDetached() (in /usr/lib/libxml-security-c.so.16.0.1)
> ==7410== by 0x444877F: XENCCipherImpl::decryptElement() (in /usr/lib/libxml-security-c.so.16.0.1)
> ==7410== by 0x44499E6: XENCCipherImpl::decryptElement(xercesc_3_1::DOMElement*) (in /usr/lib/libxml-security-c.so.16.0.1)
> ==7410== by 0x804A389: decryptXmlFile(xercesc_3_1::DOMDocument*, XSECCryptoKey*) (reproducer.cpp:93)
> ==7410== by 0x804A560: main (reproducer.cpp:119)
> ==7410==
> ==7410==
> ==7410== HEAP SUMMARY:
> ==7410== in use at exit: 44 bytes in 1 blocks
> ==7410== total heap usage: 7,284 allocs, 7,283 frees, 1,579,829 bytes allocated
> ==7410==
> ==7410== LEAK SUMMARY:
> ==7410== definitely lost: 0 bytes in 0 blocks
> ==7410== indirectly lost: 0 bytes in 0 blocks
> ==7410== possibly lost: 0 bytes in 0 blocks
> ==7410== still reachable: 44 bytes in 1 blocks
> ==7410== suppressed: 0 bytes in 0 blocks
> ==7410== Rerun with --leak-check=full to see details of leaked memory
> ==7410==
> ==7410== For counts of detected and suppressed errors, rerun with: -v
> ==7410== Use --track-origins=yes to see where uninitialised values come from
> ==7410== ERROR SUMMARY: 5 errors from 1 contexts (suppressed: 41 from 8)
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira