utf8_toUtf8 cannot properly handle exhausting buffer

utf8_toUtf8(const ENCODING *UNUSED_P(enc),
            const char **fromP, const char *fromLim,
            char **toP, const char *toLim)
{
  char *to;
  const char *from;
  const char *fromLimInitial = fromLim;

  /* Avoid copying partial characters. */
  align_limit_to_full_utf8_characters(*fromP, &fromLim);

  for (to = *toP, from = *fromP; (from < fromLim) && (to < toLim); from++, to++)
    *to = *from;
  *fromP = from;
  *toP = to;

  if (fromLim < fromLimInitial)
    return XML_CONVERT_INPUT_INCOMPLETE;
  else if ((to == toLim) && (from < fromLim))
    // <===== Bug is here. In case (to == toLim), it's possible that
    //        from is still pointing to partial character. For example,
    //        a character with 3 bytes (A, B, C) and form is pointing to C.
    //        It means only A and B is copied to output buffer. Next
    //        scanning will start with C which could be considered as invalid
    //        byte and got dropped. After this, only "AB" is kept in memory
    //        and thus it will lead to invalid continuation byte.
    return XML_CONVERT_OUTPUT_EXHAUSTED;
  else
    return XML_CONVERT_COMPLETED;
}

xmltok.c: Avoid copying of partial characters for UTF-8 input (#115)

@tianlynn

Hi @tianlynn, thanks for the report. I'm unsure about how much of a problem this is in practice — if you can manage an XML file that triggers that bug some way that would be welcome — but your finding deserves a fix independent of that. I made ~~commit 6f31d86~~ for a first take and welcome review. The test suite failure needs a closer look and a fix still. Also, if you're okay with sharing your name for the change log please let me know what it is. Thanks!

@hartwork

Hi @hartwork , I used an XML with elements such as <row body='[Chinese characters]' />. As the body attribute contains long Chinese characters and Chinese characters are mainly consist of 3 bytes in UTF-8 encoding, it will hit the problem when output buffer needs to be expanded. Please use 'Lin Tian' as my name if you can. Thanks!

xmltok.c: Avoid copying of partial characters for UTF-8 input (#115)

Should be fixed on master now, second take, name updated as well.

What was the symptom that made you notice this bug from a user perspective? It sounds like you looked at the code after running into issues in practice. It would be awesome if we had a minimal example XML file that people could use to test and our test suite as well. Such a file and the symptom description together would allow for that. Can you help?

The symptom is a result of UnicodeDecodeError with invalid continuation type at some position. I tried to create a simplified xml for a repro but did not succeed. Let me keep working on it.

I just found that (in a way) this is regression from 0dbbf43.

repos.zip
Please check out attached repro files. I can repro it by running "python test.py" on Windows with Python 3.6.2. I did not try other OS or version. I made a fix locally and it can resulted in succeeding. Let me know if it does not repro for you. (The error in this file is UnicodeDecodeError: 'utf-8' codec can't decode bytes in position 3-4: unexpected end of data. So it could show all kinds of possible issues that are caused by partial characters, I think.)

Awesome, many thanks!

I have extended the Python code a bit and reduced the XML file in size a bit, too: repos2.zip.
Could you have a look and see if the error (despite change in position) is still the same? My output over here:

# python2 test.py
Selftest passed: File is valid UTF-8 as expected
Selftest passed: xmllint/libxml2 considers it well-formed XML
BUG xml.parsers.expat: 'utf8' codec can't decode bytes in position 1020-1021: unexpected end of data
BUG xml.etree.ElementTree: 'utf8' codec can't decode bytes in position 1020-1021: unexpected end of data

I ran your test and got the same error. Thanks for working out a simplified version!

Changes: Remove "potential" as we known better now (#115)

I have written a small C program issue115.c now to test for this issue without a layer like Python on top. It uses iconv to check for incomplete UTF-8.

Combining that code and test.xml from repos2.zip, the bug shows with Expat 2.2.1 and later. Earlier versions may be unaffected, or they just need a slightly different XML file to trigger that bug: I don't know.

This is what I see on the terminal. Note the difference in linked versions of Expat:

# gcc -std=c99 -pedantic -Wall -Wextra -lexpat issue115.c

# LD_LIBRARY_PATH=.libs ./a.out < test.xml 
INFO: We were COMPILED with Expat version 2.2.3
INFO: We are now LINKED with Expat version 2.2.0
INFO: iconv succeeded
INFO: 1023 of 1023 bytes UTF-8 input converted, 1416 bytes UTF-32 output written (354 units 4-byte wchar_t)
INFO: Converted text is "01234567890123456???????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????? ???????????????????????"
INFO: XML_Parse returned 1

# LD_LIBRARY_PATH=.libs ./a.out < test.xml 
INFO: We were COMPILED with Expat version 2.2.3
INFO: We are now LINKED with Expat version 2.2.1
INFO: iconv failed with EINVAL (which is expected with bad UTF-8)
INFO: 1020 of 1022 bytes UTF-8 input converted, 1412 bytes UTF-32 output written (353 units 4-byte wchar_t)
INFO: Converted text is "01234567890123456???????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????? ??????????????????????"
INFO: XML_Parse returned 1

If you find out why the Chinese characters appear as question marks, please let me know.

Does Python upstream know about this issue already? My vote for letting them know. If they need a new release, I can make some. Fix commit is 74a7090.

Well, I did not work on Linux and I've come the long way starting from downloading, building and installing recent gentoo before I can compile your code and I have expat 2.2.1 installed and I see exact error with 1412 bytes. I double checked that the wchar_t characters in your output is well-formed UTF-32 and I have no idea why wprintf screwed them on UTF-8 term. Due to my limited knowledge, I may not be able to answer the question regarding those question marks in short time. Will let you know when I get an answer but it will not be in a day or two.

I've logged the same issue on Python first but it's simply closed after pointing me to this github project. https://bugs.python.org/issue31170

Well, I debugged and found that strings are converted to ASCII in wprintf. I added following things to issue115.c and get proper output.

#include <locale.h>

setlocale(LC_CTYPE, "en_US.utf-8");

Then I got this:

INFO: We were COMPILED with Expat version 2.2.1
INFO: We are now LINKED with Expat version 2.2.1
INFO: iconv failed with EINVAL (which is expected with bad UTF-8)
INFO: 1020 of 1022 bytes UTF-8 input converted, 1412 bytes UTF-32 output written (353 units 4-byte wchar_t)
INFO: Converted text is "01234567890123456古人咏雪抽幽思骋妍辞竞险韵偶得一编奇绝辄擅美当时流声后代是以北门之风南山之雅梁园之简黄台之赋至今为作家称述尚矣及至洛阳之卧剡溪之兴灞桥之思亦皆传为故事钱塘沈履德先生隐居西湖两峰间孤高贞洁与雪同调方大雪满天皴肤粟背之际先生乃鹿中豹舄端居闭门或扶童曳杖踏遍六桥三竺时取古人诗讽咏之合唐宋元诸名家集句成诗得二百四十章联络通穿如出一人如呵一气气立于言表格备于篇中略无掇拾补凑之形非胸次包罗壮阔笔底驱走鲍谢欧苏诸公不能为此世称王荆公为集句擅长观其在钟山对雪仅题数篇未见有此噫嘻奇矣哉亦富矣哉予慕先生有袁安之节愧不能为慧可之立乃取新集命工传写使海内同好者知先生为博古传述之士而一新世人之耳目他日必有慕潜德阐幽光而剞劂以传者余实为之执殳矣弘治戊午仲冬望日慈溪杨子器衵于海虞官舍序毕诗"

Now, I think we can conclude this issue completely. Thank you for proactively working out the fix.

update python27 by one teeny, fixing 3 vulnerabilities.

Pullup ticket #5760 - requested by spz

update python27 by one teeny, fixing 3 vulnerabilities.

Pullup ticket #5760 - requested by spz

update python27 by one teeny, fixing 3 vulnerabilities.

Pullup ticket #5760 - requested by spz

update python27 by one teeny, fixing 3 vulnerabilities.

Navigation Menu

Repository navigation

utf8_toUtf8 cannot properly handle exhausting buffer #115

11 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

utf8_toUtf8 cannot properly handle exhausting buffer #115

Description

Activity

hartwork commented on Aug 10, 2017

tianlynn commented on Aug 11, 2017

hartwork commented on Aug 11, 2017

tianlynn commented on Aug 11, 2017

hartwork commented on Aug 12, 2017

tianlynn commented on Aug 12, 2017

hartwork commented on Aug 12, 2017

tianlynn commented on Aug 13, 2017

hartwork commented on Aug 13, 2017

tianlynn commented on Aug 14, 2017

tianlynn commented on Aug 15, 2017

tianlynn commented on Aug 15, 2017

11 remaining items

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions