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:11:48 UTC

[jira] [Created] (SANTUARIO-272) Memory bug inside XENCCipherImpl::doSerialise

Memory bug inside XENCCipherImpl::doSerialise
---------------------------------------------

                 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


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)

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (SANTUARIO-272) Memory bug inside XENCCipherImpl::deSerialise

Posted by "Paulo Zanoni (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SANTUARIO-272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paulo Zanoni updated SANTUARIO-272:
-----------------------------------

    Attachment: deSerialize.patch

Added patch mentioned in bug description.

> 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
>         Attachments: deSerialize.patch, deserializer.tar.gz
>
>
> 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

[jira] [Updated] (SANTUARIO-272) Memory bug inside XENCCipherImpl::deSerialise

Posted by "Paulo Zanoni (JIRA)" <ji...@apache.org>.
     [ 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

[jira] [Updated] (SANTUARIO-272) Memory bug inside XENCCipherImpl::deSerialise

Posted by "Paulo Zanoni (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SANTUARIO-272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paulo Zanoni updated SANTUARIO-272:
-----------------------------------

    Attachment: deserializer.tar.gz

Example program that can reproduce the bug

> 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
>         Attachments: deserializer.tar.gz
>
>
> 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

[jira] [Resolved] (SANTUARIO-272) Memory bug inside XENCCipherImpl::deSerialise

Posted by "Scott Cantor (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SANTUARIO-272?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Scott Cantor resolved SANTUARIO-272.
------------------------------------

       Resolution: Fixed
    Fix Version/s: C++ 1.6.1

http://svn.apache.org/viewvc?view=revision&revision=1125207

> 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
>             Fix For: C++ 1.6.1
>
>         Attachments: deSerialize.patch, deserializer.tar.gz
>
>
> 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