bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow

[Kamil Dudka]

On Thursday, August 17, 2017 12:14:05 Paul Eggert wrote:
> On 08/17/2017 04:40 AM, Kamil Dudka wrote:
> > -typedef short int DELTA;   /* to hold displacement within one context */
> > +typedef int DELTA;         /* to hold displacement within one context */
> 
> Thanks for the heads-up. Although that fixes things for that particular
> test case, it won't work for larger cases.

Do you have an example of the larger case?  We could add a test-case for it.

> The type should be ptrdiff_t instead of int.
> 
> As its FIXME comment says, ptx is riddled with integer-overflow bugs. I
> installed the attached patch to fix the bug that you mentioned along
> with the other low-hanging fruit that I found, and am marking the bug as
> fixed upstream. I expect some other integer-overflow bugs can still
> occur in practice, but at least this patch is a significant improvement.
> 
> This patch prefers signed integer types like ptrdiff_t to unsigned types
> like size_t, as signed types allow for better checking when compiled
> with sanitization.

Your patch introduces the following warnings:

Error: CONSTANT_EXPRESSION_RESULT:
src/ptx.c:1939: result_independent_of_operands: "tmp <= 9223372036854775807L" 
is always true regardless of the values of its operands. This occurs as the 
logical second operand of "&&".
# 1937|               intmax_t tmp;
# 1938|               if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) == 
LONGINT_OK
# 1939|->                    && 0 < tmp && tmp <= PTRDIFF_MAX))
# 1940|                 die (EXIT_FAILURE, 0, _("invalid gap width: %s"),
# 1941|                      quote (optarg));

Error: CONSTANT_EXPRESSION_RESULT:
src/ptx.c:1966: result_independent_of_operands: "tmp <= 9223372036854775807L" 
is always true regardless of the values of its operands. This occurs as the 
logical second operand of "&&".
# 1964|               intmax_t tmp;
# 1965|               if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) == 
LONGINT_OK
# 1966|->                    && 0 < tmp && tmp <= PTRDIFF_MAX))
# 1967|                 die (EXIT_FAILURE, 0, _("invalid line width: %s"),
# 1968|                      quote (optarg));

Anyway, it fixes the original bug so I am fine with the patch as it is.

Thank you for pushing the fix!

Kamil