#1724 Explicit null dereferenced
Closed: Fixed None Opened 6 years ago by jhrozek.

195errno_t sss_nss_mc_get_record(struct sss_cli_mc_ctx *ctx,
196                              uint32_t slot, struct sss_mc_rec **_rec)
197{
198    struct sss_mc_rec *rec;
CID 13122: Explicit null dereferenced (FORWARD_NULL)Assigning: "copy_rec" = 0.
199    struct sss_mc_rec *copy_rec = NULL;
200    size_t buf_size = 0;
201    size_t rec_len;
202    uint32_t b1;
203    uint32_t b2;
204    bool copy_ok;
205    int count;
206    int ret;
207
208    /* try max 5 times */
At conditional (1): "count > 0" taking the true branch.
At conditional (3): "count > 0" taking the true branch.
At conditional (5): "count > 0" taking the true branch.
209    for (count = 5; count > 0; count--) {
210        rec = MC_SLOT_TO_PTR(ctx->data_table, slot, struct sss_mc_rec);
211
212        /* fetch record length */
213        b1 = rec->b1;
214        __sync_synchronize();
215        rec_len = rec->len;
216        __sync_synchronize();
217        b2 = rec->b2;
At conditional (2): "!((b1 & 0xff000000U) == 0xf0000000U)" taking the true branch.
At conditional (4): "!((b1 & 0xff000000U) == 0xf0000000U)" taking the true branch.
At conditional (6): "!((b1 & 0xff000000U) == 0xf0000000U)" taking the false branch.
At conditional (7): "b1 != b2" taking the false branch.
218        if (!MC_VALID_BARRIER(b1) || b1 != b2) {
219            /* record is inconsistent, retry */
220            continue;
221        }
222
At conditional (8): "rec_len > buf_size" taking the false branch.
223        if (rec_len > buf_size) {
224            free(copy_rec);
225            copy_rec = malloc(rec_len);
226            if (!copy_rec) {
227                ret = ENOMEM;
228                goto done;
229            }
230            buf_size = rec_len;
231        }
232        /* we cannot access data directly, we must copy data and then
233         * access the copy */
At conditional (9): "(_b1 & 0xff000000U) == 0xf0000000U" taking the true branch.
Passing null variable "copy_rec" to function "memcpy", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)
234        MEMCPY_WITH_BARRIERS(copy_ok, copy_rec, rec, rec_len);
235
236        /* we must check data is consistent again after the copy */

This can happen only if rec_len == 0
This is bad, but normally can't happen.
However we should test for it and get out with an error if rec_len is smaller than the minimum record size or bigger than the max.
It should never happen, but should it happen (corrupt file ?) we should just bail out.

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.9.4

Fields changed

owner: somebody => pbrezina
selected: =>
status: new => assigned

Fields changed

patch: 0 => 1

resolution: => fixed
status: assigned => closed

Metadata Update from @jhrozek:
- Issue assigned to pbrezina
- Issue set to the milestone: SSSD 1.9.4

2 years ago

Login to comment on this ticket.

Metadata