[gnutls-devel] GnuTLS | Fix coverity in lib/ (!1092)

[Development of GNU's TLS library]

Tim Rühsen commented on a discussion on lib/str.c: https://gitlab.com/gnutls/gnutls/merge_requests/1092#note_229640804

>  
>  	while (pos < dest->length) {
>  		if (dest->data[pos] == '%') {
> -			char b[3];
> -			unsigned int u;
>  			unsigned char x;
>  
> -			b[0] = dest->data[pos + 1];
> -			b[1] = dest->data[pos + 2];
> -			b[2] = 0;
> -
> -			sscanf(b, "%02x", &u);
> -
> -			x = u;
> +			hex_decode((char *) dest->data + pos + 1, 2, &x, 1);

`hex_decode` is used by `gnutls_hex_decode2` and `gnutls_hex_decode` which are used at several places.

The function `_gnutls_buffer_unescape()` itself seems to be suboptimal, under worst circumstances it has a O(n^2) behavior. Technically better would be a `_gnutls_buffer_append_unescape()` which unescapes the input while adding it to the buffer (would fit since there also is `_gnutls_buffer_append_escape()`).

I prefer a unit test that tests the corner cases like incomplete/invalid escape sequences. Since that is not much, what existing unit test C file should it go into ? Also the old code adds random bytes if the escape sequence isn't valid (if sscanf() fails). What should we do if the input has e.g. something like `%Z` or `%aZ`. I suggest that we don't touch the sequence in such cases.

-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/merge_requests/1092#note_229640804
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20191012/53a8b533/attachment.html>