Linux console tools development discussion
 help / color / mirror / Atom feed
From: Alexey Gladkov <gladkov.alexey@gmail.com>
To: Linux console tools development discussion <kbd@lists.altlinux.org>
Subject: Re: [kbd] [PATCH] Validate psfu headers to avoid integer overflows.
Date: Mon, 26 Dec 2016 17:15:46 +0100
Message-ID: <20161226161546.GA22155@comp-core-i7-2640m-0182e6.fortress> (raw)
In-Reply-To: <20161120171543.GA30913@localhost>

On Sun, Nov 20, 2016 at 06:15:43PM +0100, Tobias Stoeckmann wrote:
> The psfu parser does not properly validate parsed values:
> 
> * unsigned int values are casted to signed int values when
>   parameters are supplied, therefore they must be checked against
>   INT_MAX (local size_t variables are used)
> * fontwidth must not be larger than INT_MAX - 7, otherwise later
>   alignment codes would overflow, e.g. (fontwidth + 7) / 8
> * "ftoffset + fontlen * charsize" is prone to overflow, make sure
>   that it does not; later on it will be checked against file size
> * when parsing multiple files, make sure that the sum of all
>   fonts won't overflow

I like the idea, but I don't like this patch. I consider it a bad idea to
collect all the checks in one place. Most of them looks like black magic
if you don't know the rest of source code.

Right now Oleg is developing the implementation of the library which will
replace this buggy code. I hope this library will be ready for next
release (not upcoming release).

> ---
> I've sent this mail in August 2015 already. Based on the upcoming
> release, it might be a good idea to re-evaluate it.

I guess I lost it. Sorry.

> Attached are two files which will crash the current code:
> 
> $ setfont setfont-fpe.psfu # font width too large
> Floating point exception
> $ psfxtable -i psfxtable-segfault.psfu # on 32 bit archs
> Segmentation fault

Thanks for test cases!

-- 
Rgrds, legion



      reply	other threads:[~2016-12-26 16:15 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-20 17:15 Tobias Stoeckmann
2016-12-26 16:15 ` Alexey Gladkov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161226161546.GA22155@comp-core-i7-2640m-0182e6.fortress \
    --to=gladkov.alexey@gmail.com \
    --cc=kbd@lists.altlinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux console tools development discussion

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://lore.altlinux.org/kbd/0 kbd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kbd kbd/ http://lore.altlinux.org/kbd \
		kbd@lists.altlinux.org kbd@lists.altlinux.ru kbd@lists.altlinux.com
	public-inbox-index kbd

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://lore.altlinux.org/org.altlinux.lists.kbd


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git