Skip to content

Fix out of bounds memory read in onig_node_str_cat#164

Open
sashashura wants to merge 2 commits intok-takata:masterfrom
sashashura:patch-1
Open

Fix out of bounds memory read in onig_node_str_cat#164
sashashura wants to merge 2 commits intok-takata:masterfrom
sashashura:patch-1

Conversation

@sashashura
Copy link
Copy Markdown

This PR fixes out of bounds memory read in onig_node_str_cat revealed by fuzzing fluent-bit:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=46049

The root cause is that a call to enclen inside of PFETCH macro when called in fetch_token results in a call to onigenc_mbclen_approximate.
When the value of p passed to the function is \xec even though it is the last byte in multibyte sequince (the next byte is unexpected string terminator \0) the onigenc_mbclen_approximate returns it's size as 4. The size is added to the overall string length and results in reading past the end of the string.

@sashashura
Copy link
Copy Markdown
Author

@k-takata?

@DavidKorczynski
Copy link
Copy Markdown

Given #165 and this PR, does it make sense to create a fork that accepts security patches and then use that fork?

@sashashura
Copy link
Copy Markdown
Author

I have contemplated making local temporary changes in https://github.com/fluent/fluent-bit/tree/master/lib/onigmo. Making a temporary fork for security patches only until this repository gets back to life is an option if there are more dependent repositories willing to adopt it. In any case fluent-bit maintainers have to be convinced to make a change (either local patch or reference another fork), but they hesitate to accept security patches even in their code so far.

sashashura added a commit to sashashura/fluent-bit that referenced this pull request Sep 5, 2022
Temporary fix until k-takata/Onigmo#164 is merged
sashashura added a commit to sashashura/fluent-bit that referenced this pull request Sep 5, 2022
Temporary fix until k-takata/Onigmo#164 is merged
Signed-off-by: sashashura <93376818+sashashura@users.noreply.github.com>
sashashura added a commit to sashashura/fluent-bit that referenced this pull request Sep 5, 2022
Temporary fix until k-takata/Onigmo#164 is merged
Signed-off-by: sashashura <93376818+sashashura@users.noreply.github.com>
sashashura added a commit to sashashura/onigmo-1 that referenced this pull request Sep 7, 2022
Temporary fix until k-takata/Onigmo#164 is merged
Signed-off-by: sashashura <93376818+sashashura@users.noreply.github.com>
sashashura added a commit to sashashura/onigmo-1 that referenced this pull request Sep 7, 2022
Temporary fix until k-takata/Onigmo#164 is merged
Signed-off-by: sashashura <93376818+sashashura@users.noreply.github.com>
edsiper pushed a commit to fluent/onigmo that referenced this pull request Sep 11, 2022
Temporary fix until k-takata/Onigmo#164 is merged
Signed-off-by: sashashura <93376818+sashashura@users.noreply.github.com>
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