Skip to content

Commit

Permalink
Fix overflow problem in brace expansion of ranges with increment
Browse files Browse the repository at this point in the history
Example reproducer:

    $ echo {1696512000..1696512000..300} | wc -w
       47722

Expected output: 1. Only 1 number (1696512000) should be generated.

Analysis: The issue is a 32-bit integer overflow in
path_generate():

    src/cmd/ksh93/sh/expand.c:
    250: int first = 0, last = 0, incr = 0, count = 0;
    [...]
    408: if(incr*(first+incr) > last*incr)

Line 408 has an inherent overflow problem due to the multiplication
by incr. In the reproducer, 1696512000 * 300 well exceeds INT_MAX.

This bug has been present since the range feature was introduced
in ksh 93q+ 2005-05-22 and survives in every ksh version since.

You'd think that line 408 can be simplified to:
    if(first+incr > last)
However, that doesn't work because incr may be negative. With the
too-clever-by-half multiplication eliminated, the comparison
operator (>) will now also need to adapt to the negativity of incr.

src/cmd/ksh93/sh/expand.c: path_generate():
- Fix range brace expansion to check first + incr < last if incr <
  0, or first + incr > last otherwise.

src/cmd/ksh93/sh.1:
- Formatting fixes.
- Document that n1, n2 and n3 are limited to INT_MIN...INT_MAX and
  that exceeding this range produces undefined behaviour.

Thanks to Robert W. Mills (@rwmills) for the bug report.
Resolves: #738
  • Loading branch information
McDutchie committed Apr 3, 2024
1 parent d68f78d commit f7133c5
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 17 deletions.
7 changes: 7 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ This documents significant changes in the dev branch of ksh 93u+m.
For full details, see the git log at: https://github.com/ksh93/ksh
Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.

2024-03-03:

- Fixed incorrect behaviour of {n1..n2..incr} range brace expansions, which
sometimes occurred if the value of n1 multiplied by incr falls outside of
the INT_MIN..INT_MAX range. For instance, {1696512000..1696512000..300}
now correctly outputs only 1696512000.

2024-03-30:

- A regression was fixed in ordinal specifiers in 'printf %T' date
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-03-30" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-04-03" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
30 changes: 16 additions & 14 deletions src/cmd/ksh93/sh.1
Original file line number Diff line number Diff line change
Expand Up @@ -2536,19 +2536,11 @@ as well as any fields resulting from
(see above),
are checked to see if they contain one or more of the brace patterns
.BR {*,*} ,
.BI { l1 .. l2 }
,
.BI { n1 .. n2 }
,
.BI { n1 .. n2 %
.IB fmt }
,
.BI { n1 .. n2
.BI .. n3 }
, or
.BI { n1 .. n2
.BI .. n3 % fmt }
, where
\f3{\fP\f2l1\fP\f3..\fP\f2l2\fP\f3}\fP,
\f3{\fP\f2n1\fP\f3..\fP\f2n2\fP\f3}\fP,
\f3{\fP\f2n1\fP\f3..\fP\f2n2\fP\f3%\fP\f2fmt\fP\f3}\fP,
\f3{\fP\f2n1\fP\f3..\fP\f2n2\fP\f3..\fP\f2n3\fP\f3}\fP, or
\f3{\fP\f2n1\fP\f3..\fP\f2n2\fP\f3..\fP\f2n3\fP\f3%\fP\f2fmt\fP\f3}\fP, where
.B *
represents any character,
.IR l1\^ , l2\^
Expand Down Expand Up @@ -2598,7 +2590,7 @@ with
.B *
must be quoted.
.PP
In the seconds form,
In the second form,
.I l1\^
and
.I l2\^
Expand All @@ -2617,6 +2609,16 @@ incrementing
.I n1\^
by
.IR n3\^ .
The values of
.IR n1\^ ,
.I n2\^
and
.I n3\^
are limited to the standard integer range as output by
.B "getconf INT_MIN"
and
.BR "getconf INT_MAX" ;
the behavior is undefined if this range is exceeded.
The cases where
.I n3\^
is not specified behave as if
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/sh/expand.c
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ int path_generate(struct argnod *todo, struct argnod **arghead, int musttrim)
*(rescan - 1) = '}';
*(cp = end) = 0;
}
if(incr*(first+incr) > last*incr)
if(incr < 0 ? (first + incr < last) : (first + incr > last))
*cp = '}';
else
first += incr;
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/ksh93/tests/expand.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# #
# This software is part of the ast package #
# Copyright (c) 1982-2011 AT&T Intellectual Property #
# Copyright (c) 2020-2023 Contributors to ksh 93u+m #
# Copyright (c) 2020-2024 Contributors to ksh 93u+m #
# and is licensed under the #
# Eclipse Public License, Version 2.0 #
# #
Expand Down Expand Up @@ -88,6 +88,7 @@ set -- \
'{0..10%s}' '{0..10%s}' \
'{0..10%dl}' '{0..10%dl}' \
'{a,b}{0..3%02..2u}{y,z}' 'a00y a00z a01y a01z a10y a10z a11y a11z b00y b00z b01y b01z b10y b10z b11y b11z' \
'{1696512000..1696512000..300}' '1696512000' \

while (($#>1))
do ((Line++))
Expand Down

0 comments on commit f7133c5

Please sign in to comment.