cn-uofbasel / ccn-lite

CCN-lite, a lightweight implementation of the CCNx protocol and its variations
ISC License
74 stars 63 forks source link

Parameter opts is garbage after exiting function ccnl_mkInterest #345

Open mfrey opened 5 years ago

mfrey commented 5 years ago

Description

In case a NULL pointer is passed for parameter opts of type ccnl_interest_opts_u in function ccnl_mkInterest a default value is assigned. Unfortunately, the default value is local and thus resides on the stack. Hence, the value of opts is garbage after exiting the function, i.e.

196 int8_t
197 ccnl_mkInterest(struct ccnl_prefix_s *name, ccnl_interest_opts_u *opts,
198                 uint8_t *tmp, uint8_t *tmpend, size_t *len, size_t *offs) {
199     ccnl_interest_opts_u default_opts;
200 
...
221             if (!opts) {
222                 opts = &default_opts;
223             }
...
225             if (!opts->ndntlv.nonce) {
226                 opts->ndntlv.nonce = rand();
227             }

Or am I missing something?

cgundogan commented 5 years ago

theoretically yes, but once the function ccnl_mkInterest() returns, the opts value is never used again. It's only used for marshalling the packet in ccnl_ndntlv_prependInterest. No references are saved for later use.

mfrey commented 5 years ago

theoretically yes, but once the function ccnl_mkInterest() returns, the opts value is never used again. It's only used for marshalling the packet in ccnl_ndntlv_prependInterest. No references are saved for later use.

Thanks for the heads up. Honestly, I don't really like it. I've stumbled a couple times in false positives of the static analyzer where e.g. a parameter was used in a subsequent call, etc.

We probably should check how we can silence these false positives or think about if we should clarify/fix these "issues". We also should probably add a new label which allows for easy filtering of these things (check if an issue identified by the static analyzer has been discussed before).

Any thoughts?