Project

General

Profile

Bug #8121

Dead RPC code can free unallocated memory

Added by Marcel Telka over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
2017-04-28
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

The __svcauth_sys() in libnsl(3nsl) contains this code:

82    area = (struct authsys_area *)rqst->rq_clntcred;
83    aup = &area->area_aup;
84    aup->aup_machname = area->area_machname;
85    aup->aup_gids = area->area_gids;

...

89    xdrmem_create(&xdrs, msg->rm_call.cb_cred.oa_base, auth_len,
90            XDR_DECODE);
91    buf = XDR_INLINE(&xdrs, auth_len);
92    if (buf != NULL) {

...

125    } else if (! xdr_authsys_parms(&xdrs, aup)) {
126        xdrs.x_op = XDR_FREE;
127        (void) xdr_authsys_parms(&xdrs, aup);
128        stat = AUTH_BADCRED;
129        goto done;
130    }

The aup structure is prepared to point to a preallocated memory (fields aup_machname and aup_gids at lines 84 and 85) and such structure is then passed to xdr_authsys_parms() at line 125 as a placeholder for decoded authsys_parms. In a case the xdr_authsys_parms() call at line 125 fails, we try to free the aup structure at line 127. This is wrong because there is nothing in the aup we should free. Actually, during the free attempt both aup_machname and aup_gids are freed, but they were never allocated by malloc(3c). This can cause a memory corruption.

The good news is that the scenario described above cannot happen because the code at lines 125 to 130 is just a dead code. With the current implementation of xdrmem the XDR_INLINE() call at line 91 cannot fail and return NULL.

The similar problem is in __svcauth_loopback() too.

History

#2

Updated by Marcel Telka over 2 years ago

  • Subject changed from libnsl: Dead code can free unallocated memory to Dead RPC code can free unallocated memory

I just noticed that the similar problem is in kernel too, in function _svcauth_unix().

#3

Updated by Marcel Telka over 2 years ago

  • Priority changed from Low to Normal
#4

Updated by Marcel Telka over 2 years ago

  • Status changed from In Progress to Pending RTI
#5

Updated by Vitaliy Gusev over 2 years ago

Just want to note that xdrmem_inline() can return NULL, but for this case it should return success.

#6

Updated by Electric Monk over 2 years ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 0 to 100

git commit 5806135a39e4841f6f6b7281eaf59f34fffec72e

commit  5806135a39e4841f6f6b7281eaf59f34fffec72e
Author: Marcel Telka <marcel@telka.sk>
Date:   2017-05-10T15:02:32.000Z

    8121 Dead RPC code can free unallocated memory
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Dan McDonald <danmcd@omniti.com>
    Approved by: Hans Rosenfeld <hans.rosenfeld@joyent.com>

Also available in: Atom PDF