ccn-lite icon indicating copy to clipboard operation
ccn-lite copied to clipboard

Parameter opts is garbage after exiting function ccnl_mkInterest

Open mfrey opened this issue 6 years ago • 2 comments

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?

mfrey avatar Feb 01 '19 18:02 mfrey

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.

cgundogan avatar Feb 01 '19 19:02 cgundogan

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?

mfrey avatar Feb 04 '19 08:02 mfrey