Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICU-21537 Fix invalid free by long locale name #1656

Merged

Conversation

FrankYFTang
Copy link
Contributor

@FrankYFTang FrankYFTang commented Mar 17, 2021

Only free baseName when it is neither the same as fullName nor fullNameBuffer.
If we have a very long base name (say a lot of variant) and one extension, the set extension will return buffer error and
the cause Locale to reallocate buffer for fullName, but baseName is still pointing to fullNameBase.
Then later if we free baseName in the destructor we will free the memory part of the locale itself (because
it is still pointing to fullNameBuffer) . So we need to check the free of baseName not only if it is different from fullName, but also differ from fullNameBuffer.

To test

cd icu4c/source
CXXFLAGS="-fsanitize=address" CFLAGS="-fsanitize=address" ./runConfigureICU --disable-release Linux --disable-layoutex
make clean
make -j 50 tests
cd test/intl
LD_LIBRARY_PATH=lib:stubdata:tools/ctestfw:../../lib:../../stubdata:../../tools/ctestfw:$LD_LIBRARY_PATH ./intltest collate/CollationTest/TestLongLocale
Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-21537
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

Do not free baseName if it is pointing to fullNameBuffer.

Better Fix
@FrankYFTang FrankYFTang force-pushed the ICU-21537-Long-Locale-Collator branch from 521f328 to 3e8054c Compare March 17, 2021 06:12
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/common/locid.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

Copy link
Member

@markusicu markusicu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm tnx!

@FrankYFTang FrankYFTang merged commit cec7de7 into unicode-org:master Mar 17, 2021
@FrankYFTang FrankYFTang deleted the ICU-21537-Long-Locale-Collator branch March 17, 2021 17:34
daniel-ju added a commit to microsoft/icu that referenced this pull request Mar 17, 2021
This change cherry-picks an upstream fix for an invalid free of a long locale name.

Upstream ticket: https://unicode-org.atlassian.net/browse/ICU-21537
Upstream PR: unicode-org/icu#1656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants