On Wed, Jul 21, 2021 at 14:46:42 +0200, Tim Wiederhake wrote:
> virFileReadLimFD always returns null-terminated data. To that end, it has to
> add one to the maximum file size. If the maxium file size is INT_MAX, this
> triggers a signed integer overflow.
>
> There is no instance left where a caller would call virFileReadLimFD with a
> maxium file size of INT_MAX. Make virFileReadLimFD error out if the maximum
> file size is INT_MAX to prevent the reintroduction of this issue.
>
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
> src/util/virfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 723e1ca6e5..b5600658d5 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1418,7 +1418,7 @@ virFileReadLimFD(int fd, int maxlen, char **buf)
> size_t len;
> char *s;
>
> - if (maxlen <= 0) {
> + if ((maxlen <= 0) || (maxlen >= INT_MAX)) {
> errno = EINVAL;
> return -1;
While '< 0' is common sense here, limiting to INT_MAX -1 should be
mentioned in the docs.
Or better, why aren't we converting this to 'size_t' instead?
saferead_lim is already operating on 'size_t' and I think we could
simply get rid of the overflow checks altogether when working with
size_t.
