Bug #8121
closedDead RPC code can free unallocated memory
100%
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.
Updated by Marcel Telka over 6 years ago
Updated by Marcel Telka over 6 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()
.
Updated by Marcel Telka over 6 years ago
- Status changed from In Progress to Pending RTI
Updated by Vitaliy Gusev over 6 years ago
Just want to note that xdrmem_inline() can return NULL, but for this case it should return success.
Updated by Electric Monk over 6 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>