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

CVE-2021-34555: Fix multi-value From rejection logic #178

Closed
wants to merge 3 commits into from

Conversation

glts
Copy link
Contributor

@glts glts commented Jun 4, 2021

This change fixes the broken reject logic when a multi-value From header field is detected. It also adds a guard against deref of NULL domains (CVE-2021-34555, see #179).

It includes an unrelated addition, namely the missing if-guard before the syslog call, as is done elsewhere in this function.

This fixes #175.
This fixes #179.

@glts
Copy link
Contributor Author

glts commented Jun 9, 2021

This should fix the crash (DoS vuln) reported in #179. Please review and modify/merge.

I understand that you don’t have the resources to maintain this project. Still, if you can find half an hour to look this over and comment on it we can apply the patch in distros.

@glts glts changed the title Fix multi-value From rejection logic CVE-2021-34555: Fix multi-value From rejection logic Jun 16, 2021
@glts
Copy link
Contributor Author

glts commented Jun 23, 2021

We have applied this patch in Debian.

@thegushi
Copy link
Collaborator

I'll get this applied to a branch in the next day or so.

@mskucherawy
Copy link
Member

The patch contains a redundant check for "domains[0] != NULL" on new line 2477; this is already tested on line 2468.

There's a deeper problem here, which is that the caller doesn't actually know how far into that array it needs to go. The "domains" array could be 10 entries long, but with a NULL in the middle, which will short-circuit further checks.

@glts
Copy link
Contributor Author

glts commented Dec 20, 2021

The maintainers claim that this is fixed 1.4.2, closing.

@glts glts closed this Dec 20, 2021
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.

3 participants