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

lib: fix 2 Null Pointer Dereference bugs #18110

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mugitya03
Copy link

Modified Fix for #18072

We found and fixed 2 NPD bugs in lib directory. This PR has removed the superfluous commits of previous PR #18072 .

Thanks for your patience.

Function `mgmt_be_send_ds_delete_notification` passes a null value as the third argument to function __send_notification.

return __send_notification(__be_client, path, patch, NOTIFY_OP_DS_PATCH);

in function __send_notification, when both the 2nd parameter xpath and 3rd parameter tree are null, a null pointer dereference bug occurs. 

	assert(op != NOTIFY_OP_NOTIFICATION || xpath || tree);
	debug_be_client("%s: sending %sYANG %snotification: %s", __func__,
			op == NOTIFY_OP_DS_DELETE    ? "delete "
			: op == NOTIFY_OP_DS_REPLACE ? "replace "
			: op == NOTIFY_OP_DS_PATCH   ? "patch "
						     : "",
			op == NOTIFY_OP_NOTIFICATION ? "" : "DS ", xpath ?: tree->schema->name);



Fix:
Add a check that return directly when both xpath and tree are null.

Signed-off-by: mugitya03 <[email protected]>
Function vrf_get can return a NULL value.

	if (vrf && vrf_id != VRF_UNKNOWN
	    && vrf->vrf_id != VRF_UNKNOWN
	    && vrf->vrf_id != vrf_id) {
		zlog_debug("VRF_GET: avoid %s creation(%u), same name exists (%u)",
			   name, vrf_id, vrf->vrf_id);
		return NULL;
	}

In function lib_vrf_create, the return value from vrf_get is dereferenced without any null value check, causing a null pointer dereference bug. 


Fix:
We noticed that other caller functions both check the return value of function vrf_get, thus similarly, we check the return value and return NB_ERR if null.

Signed-off-by: mugitya03 <[email protected]>
@@ -944,7 +944,9 @@ static int lib_vrf_create(struct nb_cb_create_args *args)
return NB_OK;

vrfp = vrf_get(VRF_UNKNOWN, vrfname);

if (!vrfp){
Copy link
Contributor

Choose a reason for hiding this comment

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

vrfname is guaranteed to be non-NULL due to YANG schema validation.

vrf_get cannot return NULL for VRF_UNKNOWN + vrfname != NULL.

This check is therefore superfluous.

@@ -323,6 +323,9 @@ static int __send_notification(struct mgmt_be_client *client, const char *xpath,
int ret = 0;

assert(op != NOTIFY_OP_NOTIFICATION || xpath || tree);
if (!xpath && !tree){
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not correct. Due to the mgmt_msg_native_xpath_encode call below, xpath actually must be non-NULL. The assert seems to be in error.

@eqvinox
Copy link
Contributor

eqvinox commented Feb 12, 2025

Are you using AI/LLM to generate these pull requests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants