After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 758606 - (CVE-2016-1833) Heap-based buffer overread in htmlCurrentChar
(CVE-2016-1833)
Heap-based buffer overread in htmlCurrentChar
Status: RESOLVED FIXED
Product: libxml2
Classification: Platform
Component: general
git master
Other All
: Normal normal
: ---
Assigned To: Daniel Veillard
libxml QA maintainers
Depends on:
Blocks:
 
 
Reported: 2015-11-24 16:28 UTC by Mateusz Jurczyk
Modified: 2016-05-27 21:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reproducer. (258 bytes, application/octet-stream)
2015-11-24 16:28 UTC, Mateusz Jurczyk
  Details
Proposed Patch (3.42 KB, patch)
2016-02-17 01:58 UTC, pjumde
none Details | Review
Proposed Patch v2 (2.95 KB, patch)
2016-03-01 23:33 UTC, David Kilzer
none Details | Review
Reduced test case (15 bytes, text/html)
2016-03-15 00:04 UTC, David Kilzer
  Details
Proposed Patch v3 (12.39 KB, patch)
2016-03-15 22:56 UTC, David Kilzer
none Details | Review
Proposed Patch v3 --ignore-space-change (1.67 KB, patch)
2016-03-15 22:58 UTC, David Kilzer
none Details | Review

Description Mateusz Jurczyk 2015-11-24 16:28:41 UTC
Created attachment 316183 [details]
Reproducer.

Hi,

The following crash due to a heap-based out-of-bounds memory read can be observed in an ASAN build of latest stable libxml2 (2.9.3, released 4 days ago), by feeding a malformed file to xmllint ("$ ./xmllint --html /path/to/file"):

=================================================================
==26202==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62100001c900 at pc 0x0000008073f9 bp 0x7ffd791c7f90 sp 0x7ffd791c7f88
READ of size 1 at 0x62100001c900 thread T0
    #0 0x8073f8 in htmlCurrentChar libxml2-2.9.3/HTMLparser.c:439:6
    #1 0x80ee62 in htmlParseCharDataInternal libxml2-2.9.3/HTMLparser.c:3011:8
    #2 0x821b85 in htmlParseCharData libxml2-2.9.3/HTMLparser.c:3061:5
    #3 0x7df875 in htmlParseContentInternal libxml2-2.9.3/HTMLparser.c:4634:3
    #4 0x7e2f0f in htmlParseDocument libxml2-2.9.3/HTMLparser.c:4769:5
    #5 0x802c55 in htmlDoRead libxml2-2.9.3/HTMLparser.c:6741:5
    #6 0x8030b6 in htmlReadFile libxml2-2.9.3/HTMLparser.c:6799:13
    #7 0x4f47a5 in parseAndPrintFile libxml2-2.9.3/xmllint.c:2248:8
    #8 0x4ebe8f in main libxml2-2.9.3/xmllint.c:3759:7

0x62100001c900 is located 0 bytes to the right of 4096-byte region [0x62100001b900,0x62100001c900)
allocated by thread T0 here:
    #0 0x4b8b68 in malloc llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40
    #1 0xa01a0c in xmlBufCreate libxml2-2.9.3/buf.c:137:32
    #2 0x550aca in xmlSwitchInputEncodingInt libxml2-2.9.3/parserInternals.c:1205:34
    #3 0x54f5ce in xmlSwitchToEncodingInt libxml2-2.9.3/parserInternals.c:1281:12
    #4 0x54f278 in xmlSwitchEncoding libxml2-2.9.3/parserInternals.c:1101:11
    #5 0x808eea in htmlCurrentChar libxml2-2.9.3/HTMLparser.c:518:13
    #6 0x804a38 in htmlParseNameComplex libxml2-2.9.3/HTMLparser.c:2496:9
    #7 0x7cc29d in htmlParseName libxml2-2.9.3/HTMLparser.c:2483:12
    #8 0x7ec211 in htmlParseDocTypeDecl libxml2-2.9.3/HTMLparser.c:3424:12
    #9 0x7debf4 in htmlParseContentInternal libxml2-2.9.3/HTMLparser.c:4585:3
    #10 0x7e2f0f in htmlParseDocument libxml2-2.9.3/HTMLparser.c:4769:5
    #11 0x802c55 in htmlDoRead libxml2-2.9.3/HTMLparser.c:6741:5
    #12 0x8030b6 in htmlReadFile libxml2-2.9.3/HTMLparser.c:6799:13
    #13 0x4f47a5 in parseAndPrintFile libxml2-2.9.3/xmllint.c:2248:8
    #14 0x4ebe8f in main libxml2-2.9.3/xmllint.c:3759:7

SUMMARY: AddressSanitizer: heap-buffer-overflow libxml2-2.9.3/HTMLparser.c:439:6 in htmlCurrentChar
Shadow bytes around the buggy address:
  0x0c427fffb8d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fffb8e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fffb8f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fffb900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c427fffb910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c427fffb920:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fffb930: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fffb940: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fffb950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fffb960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c427fffb970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==26202==ABORTING
Comment 1 Mateusz Jurczyk 2015-11-24 19:17:20 UTC
Note, this bug is reported under the Google Project Zero framework, and is filed at https://code.google.com/p/google-security-research/issues/detail?id=636 on our side.

It is subject to a 90 day disclosure deadline. If 90 days elapse without a broadly available patch, then the bug report will automatically become visible to the public.
Comment 2 Mateusz Jurczyk 2015-11-27 16:00:31 UTC
Just realized this bug entry is already open to the public. Please disregard the deadline disclaimer.
Comment 3 pjumde 2016-02-17 01:58:02 UTC
Created attachment 321455 [details] [review]
Proposed Patch

Patch with reduced test case.
Comment 4 David Kilzer 2016-03-01 23:33:24 UTC
Created attachment 322817 [details] [review]
Proposed Patch v2

Updated patch to move test case into test/HTML since this is an HTML test.
Comment 5 David Kilzer 2016-03-15 00:04:35 UTC
Created attachment 323938 [details]
Reduced test case

This is a reduced test case from Attachment #316183 [details].

However, the patch posted doesn't address the original test case.
Comment 6 David Kilzer 2016-03-15 22:56:43 UTC
Created attachment 324062 [details] [review]
Proposed Patch v3

This addresses both the reduced test case and the original test case.

It also contains a second reduced test case.
Comment 7 David Kilzer 2016-03-15 22:58:57 UTC
Created attachment 324063 [details] [review]
Proposed Patch v3 --ignore-space-change

This patch shows the changes to parserInternals.c without the space changes of Attachment 324062 [details] using `git diff --ignore-space-change'.

It's much easier to see what changed here than the patch that applies to the tree.
Comment 8 David Kilzer 2016-03-16 17:33:46 UTC
(In reply to David Kilzer from comment #6)
> Created attachment 324062 [details] [review] [review]
> Proposed Patch v3
> 
> This addresses both the reduced test case and the original test case.
> 
> It also contains a second reduced test case.

Note for people back-porting this patch (as attached):

The assert() added in this patch is hit when running the XML conformance test suite at <http://www.w3.org/XML/Test/xmlts20080827.tar.gz>.  To do that, unpack the archive into the libxml2 source directory, then run "make check" or "./runxmlconf".

This bug was fixed in v2.9.3 by fdfeecc1b73b0318466f0d61f0b8881ed9d92dd2, so you may want to merge that with this patch:
<https://git.gnome.org/browse/libxml2/commit/?id=fdfeecc1b73b0318466f0d61f0b8881ed9d92dd2>
Comment 9 Daniel Veillard 2016-05-23 02:08:33 UTC
Okay, queued for push, but I removed the assert and used an error message and
halting the parser, the library can't just exit on hitting an error
even an internal one.

Staged for push,

  thanks !

Daniel