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

Buffer overflow fix as NULL terminator wasn't copied in PCRE invocation. #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rsdcastro
Copy link

Adding "+ 1" to allocation includes NULL terminator to avoid buffer overflow
as PCRE library runs strlen on the string and requires the string to be
properly terminated by NULL.

Adding "+ 1" to allocation includes NULL terminator to avoid buffer overflow
as PCRE library runs strlen on the string and requires the string to be
properly terminated by NULL.
@jordansissel
Copy link
Owner

pcre_exec's 4th argument is the length of the string, in bytes, so it does not need strlen().

Do you have an example where this crashes? Or a stack trace?

@rsdcastro
Copy link
Author

Actually it's not the pcre_exec() which is the problem, but pcre_compile() used in grok_compilen().

From grokre.c (grok_compilen()):
grok->full_pattern = grok_pattern_expand(grok);

if (grok->full_pattern == NULL) {
grok_log(grok, LOG_COMPILE, "A failure occurred while compiling '%.*s'",
length, pattern);
grok->errstr = "failure occurred while expanding pattern "
"(too pattern recursion?)";
return GROK_ERROR_COMPILE_FAILED;
}

grok->re = pcre_compile(grok->full_pattern, 0,
&grok->pcre_errptr, &grok->pcre_erroffset, NULL);

Here pcre_compile runs a strlen on the full_pattern. Since full_pattern is allocated in grok_pattern_expand, I fixed the allocation there.

From pcre_compile.c (lines 5487 and 5488) - I'm using pcre 7.3:

cd->start_pattern = (const uschar *)pattern;
cd->end_pattern = (const uschar *)(pattern + strlen(pattern));

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