tree 720df684a84090b2ca59146cf008405e09f73b3c
parent 99645ff9eecbf2178199aa940703fbe8ed275867
author Tobias Stoeckmann <tobias@stoeckmann.org> 1466875133 +0200
committer Akira TAGOH <akira@tagoh.org> 1470375305 +0900

Properly validate offsets in cache files.

The cache files are insufficiently validated. Even though the magic
number at the beginning of the file as well as time stamps are checked,
it is not verified if contained offsets are in legal ranges or are
even pointers.

The lack of validation allows an attacker to trigger arbitrary free()
calls, which in turn allows double free attacks and therefore arbitrary
code execution. Due to the conversion from offsets into pointers through
macros, this even allows to circumvent ASLR protections.

This attack vector allows privilege escalation when used with setuid
binaries like fbterm. A user can create ~/.fonts or any other
system-defined user-private font directory, run fc-cache and adjust
cache files in ~/.cache/fontconfig. The execution of setuid binaries will
scan these files and therefore are prone to attacks.

If it's not about code execution, an endless loop can be created by
letting linked lists become circular linked lists.

This patch verifies that:

- The file is not larger than the maximum addressable space, which
  basically only affects 32 bit systems. This allows out of boundary
  access into unallocated memory.
- Offsets are always positive or zero
- Offsets do not point outside file boundaries
- No pointers are allowed in cache files, every "pointer or offset"
  field must be an offset or NULL
- Iterating linked lists must not take longer than the amount of elements
  specified. A violation of this rule can break a possible endless loop.

If one or more of these points are violated, the cache is recreated.
This is current behaviour.

Even though this patch fixes many issues, the use of mmap() shall be
forbidden in setuid binaries. It is impossible to guarantee with these
checks that a malicious user does not change cache files after
verification. This should be handled in a different patch.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>