The oc_do_*_multicast functions use pointer to out of scope endpoint
Due to the possibility that this could be a security issue I am marking this as a critical issue. If upon review it is not considered critical feel free to change the issue severity.
The following functions appear to return a pointer to an out of scope endpoint:
bool oc_do_ip_multicast(const char *uri, const char *query,
oc_response_handler_t handler, void *user_data);
bool oc_do_realm_local_ipv6_multicast(const char *uri, const char *query,
oc_response_handler_t handler,
void *user_data);
bool oc_do_site_local_ipv6_multicast(const char *uri, const char *query,
oc_response_handler_t handler,
void *user_data);
This issue was found with manual code inspection while investigating the life time of oc_endpoint_t pointers.
I will describe the issue by only looking at {{oc_do_ip_multicast}} this function is quite simple only a few lines of code.
bool
oc_do_ip_multicast(const char *uri, const char *query,
oc_response_handler_t handler, void *user_data)
{
oc_client_cb_t *cb4 = NULL;
#ifdef OC_IPV4
cb4 = oc_do_ipv4_multicast(uri, query, handler, user_data);
#endif /* OC_IPV4 */
return multi_scope_ipv6_multicast(cb4, 0x02, uri, query, handler, user_data);
}
if IPv4 is supported {{oc_do_ipv4_multicast}} is called. The first few lines of code are where the local parameter is passed to the {{oc_response_handler_t}} as a pointer.
static oc_client_cb_t *
oc_do_ipv4_multicast(const char *uri, const char *query,
oc_response_handler_t handler, void *user_data)
{
oc_client_handler_t client_handler;
client_handler.response = handler;
oc_make_ipv4_endpoint(mcast4, IPV4 | DISCOVERY, 5683, 0xe0, 0x00, 0x01, 0xbb); oc_client_cb_t *cb = oc_ri_alloc_client_cb(
uri, &mcast4, OC_GET, query, client_handler, LOW_QOS, user_data);
The macro oc_make_ipv4_endpoint is expanded to a local variable with the name
mcast4
expaned macro would be this
oc_endpoint_t mcast4 = {.flags = IPV4 | DISCOVERY,
.addr.ipv4 = {.port = 5683,
.address = { 0xe0, 0x00, 0x01, 0xbb } } }
the address of the mcast4 variable is passed to the {{oc_client_cb_t *cb}} via the {{oc_ri_alloc_client_cb}} function. Inside the {{oc_ri_alloc_client_cb}} the endpoint is not copied only the pointer is added to the callback.
oc_ri.c: line 1466 (inside function oc_ri_alloc_client_cb)
cb->endpoint = endpoint; // cb->endpoint = &mcast4;
Once {{oc_do_ipv4_multicast}} returns the local scope variable {{mcast4}} is no longer a valid variable since it can only be used in the local scope. Thus the endpoint passed to the {{oc_response_handler_t}} callback is also invalid.
The {{multi_scope_ipv6_multicast}} function is then called and it repeats the same issue
oc_make_ipv6_endpoint(mcast, IPV6 | DISCOVERY, 5683, 0xff, scope, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0x01, 0x58);
mcast.addr.ipv6.scope = 0; oc_client_handler_t client_handler;
client_handler.response = handler;
oc_client_cb_t *cb = oc_ri_alloc_client_cb(
uri, &mcast, OC_GET, query, client_handler, LOW_QOS, user_data);
This time the local address of the {{mcast}} variable, the ipv6 endpoint, is passed to the {{oc_client_cb_t *cb}} via the {{oc_ri_alloc_client_cb}} function. Once {{multi_scope_ipv6_multicast}} returns the local scope variable {{mcast}} is no longer a valid endpoint. Thus the endpoint passed to the {{oc_response_handler_t}} callback is also invalid.
This possibly could be a security issue since {{oc_obt.c}} function {{discover_owned_devices}} tries to copy this endpoint via the {{cache_new_device}} function.
JIRA migration meta data
- JIRA Issue ID: LITE-95
- Reporter: georgen
- Assignee: kmaloor
- Creator: georgen
- Created at: 2019-10-29T12:56:05.000-0700
- Found in Version: master
- Fix in Version: None
- Issue Severity: Critical
- Reproducibility: Always (100%)
- Operating System: None
- Hardware/ OEM Platform: None
- External URL: None
- Bugzilla ID: None
- Product: None
- Status: To Do
- Components: base stack,security
- Priority: P2
- Due Date: None
-
Issue Type: Bug
END of JIRA migration meta data