On Wed, May 29, 2019 at 1:10 AM Young Xiao <92siuy...@gmail.com> wrote:
>
> The TCP option parsing routines in tcp_parse_options function could
> read one byte out of the buffer of the TCP options.
>
> 1 while (length > 0) {
> 2 int opcode = *ptr++;
> 3 int opsize;
> 4
> 5 switch (opcode) {
> 6 case TCPOPT_EOL:
> 7 return;
> 8 case TCPOPT_NOP: /* Ref: RFC 793 section 3.1 */
> 9 length--;
> 10 continue;
> 11 default:
> 12 opsize = *ptr++; //out of bound access
>
> If length = 1, then there is an access in line2.
> And another access is occurred in line 12.
> This would lead to out-of-bound access.
>
> Therefore, in the patch we check that the available data length is
> larger enough to pase both TCP option code and size.
>
> Signed-off-by: Young Xiao <92siuy...@gmail.com>
> ---
> net/ipv4/tcp_input.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 20f6fac..9775825 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3791,6 +3791,8 @@ void tcp_parse_options(const struct net *net,
> length--;
> continue;
> default:
> + if (length < 2)
> + return;
> opsize = *ptr++;
> if (opsize < 2) /* "silly options" */
> return;
In practice we are good, since we have at least 320 bytes of room there,
and the test done later catches silly options.
if (opsize < 2) /* "silly options" */
return;
if (opsize > length) /* remember, opsize >= 2 here */
return; /* don't parse partial options */
I guess adding yet another conditional will make this code obviously
correct for all eyes
and various tools.
Thanks.
Signed-off-by: Eric Dumazet <eduma...@google.com>
