diff options
| author | Guillaume Nault <g.nault@alphalink.fr> | 2018-11-07 19:28:56 +0100 |
|---|---|---|
| committer | Dmitry Kozlov <xeb@mail.ru> | 2018-11-12 17:00:34 +0300 |
| commit | 29b90105499d03957a63c0efb22a7852b2b1faa1 (patch) | |
| tree | 46d6327e6a9b4e360032034c1ad657ac6032690b /accel-pppd | |
| parent | 142c943721615020bca80de4c69e6bbf574529aa (diff) | |
| download | accel-ppp-29b90105499d03957a63c0efb22a7852b2b1faa1.tar.gz accel-ppp-29b90105499d03957a63c0efb22a7852b2b1faa1.zip | |
ipcp: fix uninitialised memory access when negociating *-DNS-Address
When handling the EV_DNS event, IPCP assumes that the ->dns1 and ->dns2
fields of the event structure are properly set. But that may not be the
case.
If only one of the MS-Primary-DNS-Server or MS-Secondary-DNS-Server
RADIUS attributes was received, then only ->dns1 or ->dns2 is set,
while the other keeps a non initialised value. This uninitialised value
is then copied by ev_dns() and proposed to the peer when negociating
the Primary-DNS-Address or Secondary-DNS-Address IPCP options.
That leaks four bytes of the stack to the network and prevents using
the values found in the [dns] section of accel-ppp.conf as fallback.
Fix this by initialising the whole event structure in rad_proc_attrs().
Then, in ev_dns(), we can check if ->dns1 or ->dns2 is properly set
before copying them. That allows to propery fallback to accel-ppp.conf
values when one of the values was not provided by RADIUS.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Diffstat (limited to 'accel-pppd')
| -rw-r--r-- | accel-pppd/ppp/ipcp_opt_dns.c | 12 | ||||
| -rw-r--r-- | accel-pppd/radius/radius.c | 3 |
2 files changed, 9 insertions, 6 deletions
diff --git a/accel-pppd/ppp/ipcp_opt_dns.c b/accel-pppd/ppp/ipcp_opt_dns.c index f4028448..56160fe0 100644 --- a/accel-pppd/ppp/ipcp_opt_dns.c +++ b/accel-pppd/ppp/ipcp_opt_dns.c @@ -151,11 +151,15 @@ static void ev_dns(struct ev_dns_t *ev) ppp = container_of(ev->ses, typeof(*ppp), ses); - dns_opt = container_of(ipcp_find_option(ppp, &dns1_opt_hnd), typeof(*dns_opt), opt); - dns_opt->addr = ev->dns1; + if (ev->dns1) { + dns_opt = container_of(ipcp_find_option(ppp, &dns1_opt_hnd), typeof(*dns_opt), opt); + dns_opt->addr = ev->dns1; + } - dns_opt = container_of(ipcp_find_option(ppp, &dns2_opt_hnd), typeof(*dns_opt), opt); - dns_opt->addr = ev->dns2; + if (ev->dns2) { + dns_opt = container_of(ipcp_find_option(ppp, &dns2_opt_hnd), typeof(*dns_opt), opt); + dns_opt->addr = ev->dns2; + } } static void load_config(void) diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c index 6c3c8ee8..5e8196d5 100644 --- a/accel-pppd/radius/radius.c +++ b/accel-pppd/radius/radius.c @@ -148,14 +148,13 @@ out_err: int rad_proc_attrs(struct rad_req_t *req) { + struct ev_dns_t dns = {}; struct rad_attr_t *attr; struct ipv6db_addr_t *a; - struct ev_dns_t dns; struct ev_wins_t wins; int res = 0; struct radius_pd_t *rpd = req->rpd; - dns.ses = NULL; wins.ses = NULL; req->rpd->acct_interim_interval = conf_acct_interim_interval; |