Bug 25704 - compiler DSE pass would remove the memset, leading to memory disclosure vulnerability.
Summary: compiler DSE pass would remove the memset, leading to memory disclosure vulne...
Status: UNCONFIRMED
Alias: None
Product: glibc
Classification: Unclassified
Component: network (show other bugs)
Version: unspecified
: P2 critical
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-21 15:17 UTC by shqking
Modified: 2020-03-21 15:23 UTC (History)
0 users

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description shqking 2020-03-21 15:17:14 UTC
In the glibc master branch, at sunrpc/key_call.c file, the memset at line 105 would be removed by compiler DSE pass at O1 or higher optimization level, making the developer's intention (i.e. avoid leaving secret key in memory) failing.


  86 /* key_secretkey_is_set() returns 1 if the keyserver has a secret key
  87  * stored for the caller's effective uid; it returns 0 otherwise
  88  *
  89  * N.B.:  The KEY_NET_GET key call is undocumented.  Applications shouldn't
  90  * be using it, because it allows them to get the user's secret key.
  91  */
  92 int
  93 key_secretkey_is_set (void)
  94 {
  95   struct key_netstres kres;
  96 
  97   memset (&kres, 0, sizeof (kres));
  98   if (key_call ((u_long) KEY_NET_GET, (xdrproc_t) xdr_void,
  99                 (char *) NULL, (xdrproc_t) xdr_key_netstres,
 100                 (char *) &kres) &&
 101       (kres.status == KEY_SUCCESS) &&
 102       (kres.key_netstres_u.knet.st_priv_key[0] != 0))
 103     {
 104       /* avoid leaving secret key in memory */
 105       memset (kres.key_netstres_u.knet.st_priv_key, 0, HEXKEYBYTES);
 106       return 1;
 107     }
 108   return 0;
 109 }


Note that the USENIX Security 2017 paper "Dead Store Elimination (Still) Considered Harmful" (https://www.usenix.org/conference/usenixsecurity17/technical-sessions/presentation/yang) gives a deep analysis on this problem.

We should use memset_s or other library function to mitigate this problem.
Several mitigations have been discussed in the Security 2017 paper.
Comment 1 shqking 2020-03-21 15:23:01 UTC
A similar bug is found at sunrpc/des_impl.c file, in _des_crypt() function.
The memset at line 590 would be removed by compiler as well.
It is very dangerous since this the core crypt function for DES.

 572               tout0 = tbuf[0] ^ xor0;
 573               tout1 = tbuf[1] ^ xor1;
 574               xor0 = tin0;
 575               xor1 = tin1;
 576             }
 577           else
 578             {
 579               tout0 = tbuf[0];
 580               tout1 = tbuf[1];
 581             }
 582           l2c (tout0, out);
 583           l2c (tout1, out);
 584         }
 585       l2c (tin0, oiv);
 586       l2c (tin1, oiv);
 587     }
 588   tin0 = tin1 = tout0 = tout1 = xor0 = xor1 = 0;
 589   tbuf[0] = tbuf[1] = 0;
 590   memset (schedule, 0, sizeof (schedule));
 591 
 592   return (1);
 593 }