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

Garbage value returned to caller in func NetMLDP_HostGrpLeave #33

Open
zaimingli opened this issue Mar 9, 2022 · 2 comments
Open

Garbage value returned to caller in func NetMLDP_HostGrpLeave #33

zaimingli opened this issue Mar 9, 2022 · 2 comments

Comments

@zaimingli
Copy link

zaimingli commented Mar 9, 2022

Hi everyone,

I found the return value of function NetMLDP_HostGrpLeave will be an uninitialized variable if users call this function without occupying the Net_GlobalLock and parameter p_addr is a null pointer.

In detail, assuming that the branch judgment condition on line 566 is satisfied, then the function will goto exit_release and return host_grp_leave, which is not initialized anywhere. The caller will get dirty garbage, leading to some unexpected results...

This is a public API, the wrong return value may mislead the user. Looking forward to your reply!

@yasosa305
Copy link
Member

Hi @zaimingli. Yes, this is a bug. Thank you for bring this to our attention. The solution is to initialize host_grp_leave to DEF_FAIL where we validate the IPv6 address pointer before branching to exit_release.

@zaimingli
Copy link
Author

hello @yasosa305, thank you for your reply, it encourages me a lot.

Recently I found a few more undefined behaviors, which may lead to some dangerous actions. The details are as follows:

  1. Undefined p_ix in function NetIF_TxIxDataGet

    In the net_if module, L8332 defined the var data_ix without initialization, and if the function goes to case NET_TRANSACTION_TX, a dirty address will be passed into the function NetIF_TxIxDataGet. So in L5304, p_if_api->GetPktSizeHdr(p_if) will be added to an undefined data.

  2. Undefined p_err in function NetIGMP_TxMsgReport

    net_igmp.c#L751 defines the variable err without initialization, and is passed to NetIGMP_TxMsgReport by L813. In this sequence, err is not assigned a value. In method NetIGMP_TxMsgReport, err is renamed to the local variable p_err and becomes the left value of the expression at L1822. Obviously, the left value of a function cannot be undefined behavior, and it may cause the function to return early.

  3. The final undefined behavior triggered is the same as 2, but this error is caused by a call to function NetIGMP_TxMsgReportat line L2354 of function NetIGMP_HostGrpReportDlyTimeout .

  4. Undefined return value in function NetMLDP_HostGrpLeaveHandler

    net_mldp.c#L644 defines the return value result and does not initialize any values. Assuming that the function is executed sequentially and does not exit early after any goto statements, then at L723 the function will return the variable result directly. In the process, the variable result is not assigned a value.

  5. Undefined if_nbr_src_addr in function NetNDP_NextHop

    L1267 defines the return value if_nbr_src_addr and does not initialize any values. Suppose the judgment condition in line 1312 does not satisfy, then when the program runs to line 1379, the left value if_nbr_src_addr of the expression will be a garbage value.

  6. Undefined src_addrv4 in function NetTCP_TxConnSync

    In the IPv6/net_ndp.c module, L20574 defines the return value src_addrv4 and does not initialize any values. Assuming the condition in line 20697 is not satisfied and the variable src_addrv4 will not be assigned, then in line 20832, pseg_sync_hdr->IP_AddrSrc is given the garbage data (NET_IPv4_ADDR) src_addrv4.

Meanwhile, cc @wes-jmagasrevy and @jamagasr, as it seems you are the owners of the code. Looking forward to your reply, it's very important to me!

Best Regards,
Zaiming

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

No branches or pull requests

2 participants