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

ccnl_addr2ascii returns NULL pointer

Open mfrey opened this issue 6 years ago • 3 comments

RIOT maps the various logging calls of ccn-lite to printf. Below is a snippet from ccnl-riot-logging.h (in ccn-lite).

 55 #define DEBUGMSG(LVL, ...) do {       \
 56         if ((LVL)>debug_level) break;   \
 57         LOG(LVL, __VA_ARGS__);   \
 58     } while (0)

and in RIOT LOG points to log_write points to printf (see core/include/log.h in RIOT)

 #define log_write(level, ...) printf(__VA_ARGS__)

The function ccnl_addr2ascii ccn-lite returns NULL if no valid parameter was passed to it. If one embeds a call to ccnl_addr2ascii in a logging macro call and passes NULL as a argument, the function returns NULL. Subsequently, the NULL which is passed to a %s leads to undefined behaviour (most likely a crash).

This can for example observed in ccnl_fwd.c, e.g.

236     DEBUGMSG_CFWD(INFO, "  incoming interest=<%s>%s nonce=%"PRIi32" from=%s\n",
237                   ccnl_prefix_to_str((*pkt)->pfx,s,CCNL_MAX_PREFIX_SIZE),
238                   ccnl_suite2str((*pkt)->suite), nonce,
239                   ccnl_addr2ascii(from ? &from->peer : NULL));

This issue is related to #231

mfrey avatar May 04 '18 07:05 mfrey

This issue probably also applies to other logging mappings in ccn-lite.

mfrey avatar May 04 '18 07:05 mfrey

I think we should stick with "NULL" as a return value and simply remove all parts of the code which pass clearly "NULL" to the function. Any thoughts?

mfrey avatar May 08 '18 15:05 mfrey

I've removed the "bug" label. I think that the function is simply applied in a wrongful way. A developer should check if the function returns NULL before simply passing it to a format string which expects a string such as "%s".

@blacksheeep any thoughts?

mfrey avatar May 09 '18 13:05 mfrey