getsockopt: Fix DCCP_SOCKOPT_[SEND,RECV]_CSCOV
PATCH 1/1 getsockopt: Fix DCCP_SOCKOPT_[SEND,RECV]_CSCOV
David,
Please push for 2.6.21 and stable (CCed for good measure).
- Arnaldo
-----------------------------------
We were only checking if there was enough space to put the int, but left len as specified by the (malicious) user, sigh, fix it by setting len to sizeof(val) and transfering just one int worth of data, the one asked for.
Also check for negative len values.
net/dccp/proto.c +3 -1
@@ -575,7 +575,7 @@ static int do_dccp_getsockopt(struct sock *sk, int level, int optname,if (get_user (len, optlen))return -EFAULT ; -if (len <sizeof (int )) +if (len < (int )sizeof (int ))return -EINVAL ; dp =dccp_sk (sk);
@@ -589,9 +589,11 @@ static int do_dccp_getsockopt(struct sock *sk, int level, int optname, (__be32 __user *)optval, optlen);case DCCP_SOCKOPT_SEND_CSCOV : val = dp->dccps_pcslen; + len =sizeof (val);break ;case DCCP_SOCKOPT_RECV_CSCOV : val = dp->dccps_pcrlen; + len =sizeof (val);break ; case 128 ... 191: return ccid_hc_rx_getsockopt(dp->dccps_hc_rx_ccid, sk, optname,
Quoted 6 lines · Arnaldo Carvalho de Melo
David,
Please push for 2.6.21 and stable (CCed for good measure).
- Arnaldo
Quoted 8 lines
-----------------------------------
We were only checking if there was enough space to put the int, but left len as specified by the (malicious) user, sigh, fix it by setting len to sizeof(val) and transfering just one int worth of data, the one asked for.
Also check for negative len values.
Part of the issue here is possibly that we are using signed ints here and the type from userspace is socklen_t which by my quick check is unsigned on my system.
I haven't checked how this is defined on other architectures yet but if this is the case we should tidyup to remove other possible errors of this type. I'll look into this some more as time permits.
Or am I talking through a whole in my head?
Ian
Signature · 3 lines
Web: http://wand.net.nz/~iam4/ Blog: http://iansblog.jandi.co.nz WAND Network Research Group
Quoted 19 lines · Ian McDonald
Quoted 6 lines · Arnaldo Carvalho de Melo
David,
Please push for 2.6.21 and stable (CCed for good measure).
- Arnaldo
Quoted 8 lines
-----------------------------------
We were only checking if there was enough space to put the int, but left len as specified by the (malicious) user, sigh, fix it by setting len to sizeof(val) and transfering just one int worth of data, the one asked for.
Also check for negative len values.
Part of the issue here is possibly that we are using signed ints here and the type from userspace is
socklen_twhich by my quick check is unsigned on my system.
Quoted 6 lines
I haven't checked how this is defined on other architectures yet but if this is the case we should tidyup to remove other possible errors of this type. I'll look into this some more as time permits.
Or am I talking through a whole in my head?
One way or the other we are safe now, no?
- Arnaldo
Quoted 5 lines · Arnaldo Carvalho de Melo
Quoted 4 lines · Ian McDonald
Part of the issue here is possibly that we are using signed ints here and the type from userspace is
socklen_twhich by my quick check is unsigned on my system.
Quoted 9 lines
Quoted 6 lines
I haven't checked how this is defined on other architectures yet but if this is the case we should tidyup to remove other possible errors of this type. I'll look into this some more as time permits.
Or am I talking through a whole in my head?
One way or the other we are safe now, no?
Absolutely we are safe. I'm just thinking of removing possible vectors for future problems.
Ian
Signature · 3 lines
Web: http://wand.net.nz/~iam4/ Blog: http://iansblog.jandi.co.nz WAND Network Research Group