Re: [PATCH] X.509: Fix the buffer overflow in the utility function for OID string

[Takashi Iwai]

On Sat, 19 Aug 2017 06:19:44 +0200,
Lee, Chun-Yi wrote:
> 
> From:ÂTakashi Iwai <tiwai@xxxxxxx>
> 
> The sprint_oid() utility function doesn't properly check the buffer
> size that it causes that the warning in vsnprintf() be triggered.
> For example on v4.1 kernel:
> 
> [   49.612536] ------------[ cut here ]------------
> [   49.612543] WARNING: CPU: 0 PID: 2357 at lib/vsprintf.c:1867 vsnprintf+0x5a7/0x5c0()
> ...
> 
> We can trigger this issue by injecting maliciously crafted x509 cert
> in DER format. Just using hex editor to change the length of OID to
> over the length of the SEQUENCE container. For example:
> 
>     0:d=0  hl=4 l= 980 cons: SEQUENCE
>     4:d=1  hl=4 l= 700 cons:  SEQUENCE
>     8:d=2  hl=2 l=   3 cons:   cont [ 0 ]
>    10:d=3  hl=2 l=   1 prim:    INTEGER           :02
>    13:d=2  hl=2 l=   9 prim:   INTEGER           :9B47FAF791E7D1E3
>    24:d=2  hl=2 l=  13 cons:   SEQUENCE
>    26:d=3  hl=2 l=   9 prim:    OBJECT            :sha256WithRSAEncryption
>    37:d=3  hl=2 l=   0 prim:    NULL
>    39:d=2  hl=2 l= 121 cons:   SEQUENCE
>    41:d=3  hl=2 l=  22 cons:    SET
>    43:d=4  hl=2 l=  20 cons:     SEQUENCE      <=== the SEQ length is 20
>    45:d=5  hl=2 l=   3 prim:      OBJECT            :organizationName
> 	<=== the original length is 3, change the length of OID to over the length of SEQUENCE
> 
> Pawel Wieczorkiewicz reported this problem and Takashi Iwai provided
> patch to fix it by checking the bufsize in sprint_oid().
> 
> From:ÂTakashi Iwai <tiwai@xxxxxxx>
> Reported-by: Pawel Wieczorkiewicz <pwieczorkiewicz@xxxxxxxx>
> Cc: David Howells <dhowells@xxxxxxxxxx>
> Cc: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
> Cc: Takashi Iwai <tiwai@xxxxxxx>
> Cc: Pawel Wieczorkiewicz <pwieczorkiewicz@xxxxxxxx>
> Signed-off-by: "Lee, Chun-Yi" <jlee@xxxxxxxx>

I seem to have forgotten to give my sign-off:
  Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>


thanks,

Takashi

> ---
>  lib/oid_registry.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/oid_registry.c b/lib/oid_registry.c
> index 318f382..41b9e50 100644
> --- a/lib/oid_registry.c
> +++ b/lib/oid_registry.c
> @@ -142,9 +142,9 @@ int sprint_oid(const void *data, size_t datasize, char *buffer, size_t bufsize)
>  		}
>  		ret += count = snprintf(buffer, bufsize, ".%lu", num);
>  		buffer += count;
> -		bufsize -= count;
> -		if (bufsize == 0)
> +		if (bufsize <= count)
>  			return -ENOBUFS;
> +		bufsize -= count;
>  	}
>  
>  	return ret;
> -- 
> 2.10.2
>