ccnl_prefix_cmp will crash if compcnt > 0 and comp is null

[mfrey]

Description

If you set the number of components compcnt in a ccnl_prefix_s based prefix to a number larger than 0 and set the components to NULL (or let the number of components/component count mismatch), the function ccnl_prefix_cmp will crash.

374     for (i = 0; i < plen && i < nam->compcnt; ++i) {
375         comp = i < pfx->compcnt ? pfx->comp[i] : md;
376         clen = i < pfx->compcnt ? pfx->complen[i] : 32; // SHA256_DIGEST_LEN
377         if (clen != nam->complen[i] || memcmp(comp, nam->comp[i], nam->complen[i])) {

The crash will most likely happen in line 375.

Shall we consider this a bug or do we simply assume that other parts of ccn-lite will perform sanity-checks before the function will be called or wouldn't you consider it not an issue at all (having an "empty" prefix, but a positive number of components sounds a bit esoteric)?

[blacksheeep]

In fact, this is an interessting question. I think if prefix was created by the prefix_new function this error cannot occur in practice. However, I think a check to avoid that comp is not zero, would be a good think. Nevertheless, the function can also crash if len(comp) !+ compcnt, which cannot be checked, right?

[mfrey]

However, I think a check to avoid that comp is not zero, would be a good think.

To be honest, I have mixed feelings about that. Input validation is the job of a parser, so malicious packets or data needs to be handled (to some degree) by the parser. I fear that we end up adding checks everywhere ...

[blacksheeep]

so do I. We already added checks when parsing the packets, to (hopefully) avoid this scenario from happening. So, I think, we agree here, that additional checks here are not necessary. I will close the issue, in case there are new insights, we can reopen it.