Log4cxx
  1. Log4cxx
  2. LOGCXX-162

Problem printing string with embedded NULL character

        Details

        • Type: Bug Bug
        • Status: Resolved
        • Priority: Major Major
        • Resolution: Fixed
        • Affects Version/s: None
        • Fix Version/s: 0.10.0
        • Component/s: None
        • Labels:
          None
        • Environment:
          log4cxx-svn-427970
          linux / gcc

          Description

          It's exactly the problem described in:
          http://www.mail-archive.com/log4cxx-user@logging.apache.org/msg01650.html

          ---- COPY OF MESSAGE ----

          Problem printing string with embedded NULL character

          Venkatraju
          Sat, 29 Jul 2006 19:30:11 -0700

          Hi,

          Printing a string with embedded NULL character causes
          MbstowcsCharsetDecoder::decode to go into an infinite loop trying to
          consume that character.

          Test case below:
          ------
          #include <iostream>
          #include <log4cxx/basicconfigurator.h>

          using namespace log4cxx;

          int main()

          { BasicConfigurator::configure(); std::string badStr; badStr.append("hello"); badStr.append(1, '\0'); badStr.append("world"); LOG4CXX_ERROR(Logger::getRootLogger(), badStr); return 0; } --------- Fixed with this patch: --- Index: src/charsetdecoder.cpp =================================================================== --- src/charsetdecoder.cpp (revision 390980) +++ src/charsetdecoder.cpp (working copy) @@ -178,6 +178,12 @@ stat = append(out, buf); in.position(in.position() + converted); }

          +
          + if (in.remaining() > 0 && (src == 0))

          { + // Found embedded NULL in string + stat = APR_BADARG; + break; + }

          }
          return stat;
          }

          Can someone merge this into the log4cxx trunk? Please let me know if I
          need to provide more info.

          Thanks,
          Venkat

            Activity

            Hide
            Permalink
            Curt Arnold added a comment -

            Committed rev 428399.

            log4cxx's CharsetDecoder mimics java.nio.charset.CharsetDecoder. It wasn't obvious how it would handle an embedded null character, so I wrote the following unit test to determine its behavior:

            public void testDecode8() {
            final byte[] buf = new byte[]

            { 'H', 'e', 'l', 'l', 'o', ',', 0, 'W', 'o', 'r', 'l', 'd'}

            ;
            ByteBuffer src = ByteBuffer.wrap(buf);

            CharsetDecoder dec = Charset.forName("ISO-8859-1").newDecoder();

            CharBuffer greeting = CharBuffer.allocate(20);
            CoderResult stat = dec.decode(src, greeting, true);
            assertSame(CoderResult.UNDERFLOW, stat);

            stat = dec.decode(src, greeting, true);
            assertSame(CoderResult.UNDERFLOW, stat);

            assertEquals(12, src.position());
            assertEquals("Hello,\u0000World", greeting.flip().toString());
            }

            If appears that if a null byte is encountered in the source, a null character is decoded. It does not return an error condition like the patch intended to do. The patch adds a C++ equivalent of the previous test and modified the MbstowcsCharsetDecoder::decode implementation to avoid calling mbstowcs when the string starts with a null character. It is possible that the same weakness exists in the other decoders, hopefully the unit test will catch those and they can be corrected.

            Show
            Curt Arnold added a comment - Committed rev 428399. log4cxx's CharsetDecoder mimics java.nio.charset.CharsetDecoder. It wasn't obvious how it would handle an embedded null character, so I wrote the following unit test to determine its behavior: public void testDecode8() { final byte[] buf = new byte[] { 'H', 'e', 'l', 'l', 'o', ',', 0, 'W', 'o', 'r', 'l', 'd'} ; ByteBuffer src = ByteBuffer.wrap(buf); CharsetDecoder dec = Charset.forName("ISO-8859-1").newDecoder(); CharBuffer greeting = CharBuffer.allocate(20); CoderResult stat = dec.decode(src, greeting, true); assertSame(CoderResult.UNDERFLOW, stat); stat = dec.decode(src, greeting, true); assertSame(CoderResult.UNDERFLOW, stat); assertEquals(12, src.position()); assertEquals("Hello,\u0000World", greeting.flip().toString()); } If appears that if a null byte is encountered in the source, a null character is decoded. It does not return an error condition like the patch intended to do. The patch adds a C++ equivalent of the previous test and modified the MbstowcsCharsetDecoder::decode implementation to avoid calling mbstowcs when the string starts with a null character. It is possible that the same weakness exists in the other decoders, hopefully the unit test will catch those and they can be corrected.

              People

              • Assignee:
                Curt Arnold
                Reporter:
                hunger
              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development