microsoft / microsoft/msix-packaging

[BUG] Incorrect use of new and delete, address sanitizer complains

Open
#648 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Needs-Triage
Dominant language
C
Stars
1.1k
Forks
193
Avg merge
2h 16m
Merged PRs (30d)
2

Description

Project
MSIX SDK

Describe the bug
Address sanitizer found that pointer to array is deleted incorrectly. It looks like the arrays are allocated with ::operator new(size), and later deleted with ::operator delete(ptr) where size is not known. This results in the address sanitizer reports like the one below:

   object passed to delete has wrong type:
   4   │   size of the allocated type:   34 bytes;
   5   │   size of the deallocated type: 1 bytes.

    #0 0x557eb3c0496f in operator delete(void*, unsigned long) (test_executable+0x2df196f)
   7   │     #1 0x557eb4d47ccb in MSIX::XercesXMLBytePtr::InternalRelease() .../src/msix/PAL/XML/xerces-c/XmlObject.cpp:226
   8   │     #2 0x557eb4d47c65 in MSIX::XercesXMLBytePtr::~XercesXMLBytePtr() .../src/msix/PAL/XML/xerces-c/XmlObject.cpp:215
   9   │     #3 0x557eb4d48e1f in MSIX::XercesElement::GetBase64DecodedAttributeValue(XmlAttributeName) .../src/msix/PAL/XML/xerces-c/XmlObject.cpp:265
...
  43   │ allocated by thread T0 here:
  44   │     #0 0x557eb3c03907 in operator new(unsigned long) (test_executable+0x2df0907)
  45   │     #1 0x557eb4e5dfd2 in xercesc_3_2::getExternalMemory(xercesc_3_2::MemoryManager*, unsigned long) (test_executable+0x404afd2)
  46   │     #2 0x557eb4e5eba8 in xercesc_3_2::Base64::decode(unsigned char const*, unsigned long*, unsigned char*&, xercesc_3_2::MemoryManager*, xercesc_3_2::Base64::Conformance) (test_executable+0x404bba8)
  47   │     #3 0x557eb4e5e581 in xercesc_3_2::Base64::decode(unsigned char const*, unsigned long*, xercesc_3_2::MemoryManager*, xercesc_3_2::Base64::Conformance) (test_executable+0x404b581)
  48   │     #4 0x557eb4e5e6be in xercesc_3_2::Base64::decodeToXMLByte(char16_t const*, unsigned long*, xercesc_3_2::MemoryManager*, xercesc_3_2::Base64::Conformance) (test_executable+0x404b6be)
  49   │     #5 0x557eb4d48c3b in MSIX::XercesElement::GetBase64DecodedAttributeValue(XmlAttributeName) .../src/msix/PAL/XML/xerces-c/XmlObject.cpp:265
...
 SUMMARY: AddressSanitizer: new-delete-type-mismatch (test_executable+0x2df196f) in operator delete(void*, unsigned long)
  77   │ ==23071==HINT: if you don't care about these errors you may set ASAN_OPTIONS=new_delete_type_mismatch=0
  78   │ ==23071==ABORTING

To Reproduce
Let address sanitizer look at this code:

        XercesXMLBytePtr decodedData(XERCES_CPP_NAMESPACE::Base64::decodeToXMLByte("dGVzdAo=", &len));

Expected behavior
Address sanitizer does not complain. Correct new and delete are used to allocate and deallocate memory.

Screenshots
N/A

Platform
Linux

Additional context
This happens on Linux, where MSIX packaging is using Xerces-c as its XML backend.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/msix/PAL/XML/xerces-c/XmlObject.cpp, especially XercesXMLBytePtr at lines 215 and 226 and the decoding call around line 265. Reproduce the issue with AddressSanitizer using the provided Base64::decodeToXMLByte example on Linux. Done means the allocation and deallocation no longer produce a new-delete-type-mismatch report.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, xml
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.