search

trusteddomainproject / OpenDMARC

This is the Trusted Domain Project's impementation of the DMARC protocol libary and mail filter, called OpenDMARC. A "milter" connects to unix-based mailers (originally, sendmail, but now many) and provides a standard filtering API.
Other
98 stars 52 forks source link

This is a null pointer dereference vulnerability occurring at line 1480 in the file /OpenDMARC/libopendmarc/opendmarc_policy.c #256

Open LuMingYinDetect opened 4 months ago

LuMingYinDetect commented 4 months ago

Hello, I am a graduate student specializing in static analysis of programs. Recently, while using a static analysis tool to detect issues in open-source projects, I found several defects in the project. The description of the defects can be found at the following link:https://github.com/LuMingYinDetect/OpenDMARC_defects/blob/main/OpenDMARC_detect_1.md

futatuki commented 4 months ago

Although the library function itself is obviously broken in safety, its callers in OpenDMARC, https://github.com/trusteddomainproject/OpenDMARC/blob/master/opendmarc/opendmarc-check.c#L224 and https://github.com/trusteddomainproject/OpenDMARC/blob/master/opendmarc/opendmarc.c#L3289 always call the function with size_of_buf = 0. So OpenDMARC itself is not vulnerable, I think.

futatuki commented 4 months ago

On the other hand, as a library function, opendmarc_policy_fetch_ruf() cannot owe that line_buf and size_of_buf have right values. If line_buf is not pointing right place to store some value, even if it is not NULL, the caller would not run as the programmer expected, and might also cause some violation. Thus the responsibility for line_buf and size_of_buf values should be the caller.

LuMingYinDetect commented 4 months ago

On the other hand, as a library function, opendmarc_policy_fetch_ruf() cannot owe that line_buf and size_of_buf have right values. If line_buf is not pointing right place to store some value, even if it is not NULL, the caller would not run as the programmer expected, and might also cause some violation. Thus the responsibility for line_buf and size_of_buf values should be the caller.

Thank you for your prompt reply. It seems that this defect should not be triggered. I will close the issue.

futatuki commented 4 months ago

Just wait a moment. Although it might not a vulnerability, I also think it is a bug which should be fixed (perhaps change '||' to '&&').

LuMingYinDetect commented 4 months ago

Just wait a moment. Although it might not a vulnerability, I also think it is a bug which should be fixed (perhaps change '||' to '&&').

Indeed, changing || to && will enhance the logical coherence of the program. I reopened the issue.

KIC-8462852 commented 4 months ago

Of course this is a bug that needs to be fixed. Instead of:

if (list_buf != NULL || size_of_buf > 0)

the code at libopendmarc/opendmarc_policy.c#L1478 should be:

if (list_buf != NULL && size_of_buf > 0)

You can even see this in a similar function opendmarc_policy_fetch_rua() a few lines earlier in the same source libopendmarc/opendmarc_policy.c#L1420

In the OpenDMARC project, this bug is out of reach, as opendmarc_policy_fetch_ruf() is always called with both list_buf = NULL and size_of_buf = 0:

opendmarc/opendmarc.c#L3289

ruv = opendmarc_policy_fetch_ruf(cc->cctx_dmarc, NULL, 0, TRUE);

opendmarc/opendmarc-check.c#L224

ruf = opendmarc_policy_fetch_ruf(dmarc, NULL, 0, 1);

However, this is a library function and may be used outside of this project in a way that could trigger the bug.

LuMingYinDetect commented 4 months ago

Of course this is a bug that needs to be fixed. Instead of:

if (list_buf != NULL || size_of_buf > 0)

the code at libopendmarc/opendmarc_policy.c#L1478 should be:

if (list_buf != NULL && size_of_buf > 0)

You can even see this in a similar function opendmarc_policy_fetch_rua() a few lines earlier in the same source libopendmarc/opendmarc_policy.c#L1420

In the OpenDMARC project, this bug is out of reach, as opendmarc_policy_fetch_ruf() is always called with both list_buf = NULL and size_of_buf = 0:

opendmarc/opendmarc.c#L3289

ruv = opendmarc_policy_fetch_ruf(cc->cctx_dmarc, NULL, 0, TRUE);

opendmarc/opendmarc-check.c#L224

ruf = opendmarc_policy_fetch_ruf(dmarc, NULL, 0, 1);

However, this is a library function and may be used outside of this project in a way that could trigger the bug.

Thank you for the detailed analysis! I'd like to reach out to the author of this library to address this issue. Do you have a link for submitting bugs related to this library?

KIC-8462852 commented 4 months ago

Thank you for the detailed analysis! I'd like to reach out to the author of this library to address this issue. Do you have a link for submitting bugs related to this library?

This is the place. libopendmarc is part of OpenDMARC project.