Debian Patches

Status for sgt-puzzles/20230122.806ae71-2

Patch Description Author Forwarded Bugs Origin Last update
0052-Avoid-division-by-zero-in-Cube-grid-size-checks.patch [PATCH 052/159] Avoid division by zero in Cube grid-size checks
On a triangular grid, Cube allows either d1 or d2 (but not both) to be
zero, so it's important to check that each one is not zero before
dividing by it.

The crash could be triggered by, for instance "cube t0x2".
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=df783b93e3271264a8d54f90876f41a80ef2247d 2023-02-13
0018-Mines-Don-t-check-if-the-player-has-won-if-they-ve-a.patch [PATCH 018/159] Mines: Don't check if the player has won if they've already lost

It can't happen in normal play, but if a save file had a "C" (clear
around) move that set off a mine, Mines could end up hitting an
assertion failure, "ncovered >= nmines". This was because it would
check if the player had won by counting covered squares and mines, and
of course an uncovered mine is no longer a covered square but is still a
mine.

Since winning after you're dead isn't possible (at least in Mines), we
now skip the check entirely if the player has already died.

This save file demonstrates the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :5:Mines
PARAMS :1:7
CPARAMS :1:7
DESC :22:r31,u,0000C000d0000020
NSTATES :1:2
STATEPOS:1:1
MOVE :4:C6,2
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=2a9be2b89df3e6a07a1d90a06f8ac00a92d789e5 2023-02-01
0043-Don-t-leak-grids-in-Loopy-s-validate_desc.patch [PATCH 043/159] Don't leak grids in Loopy's validate_desc() Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=11b631ea870355306c4b1d03458bb3cea8f29188 2023-02-13
0044-Remember-to-free-the-to_draw-member-from-Net-s-draws.patch [PATCH 044/159] Remember to free the to_draw member from Net's drawstate Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=493bf16ddbe2185664d6c3053f7891a9f232c75c 2023-02-13
0001-Apply-version-string-substitutions-from-the-tarball.patch Apply version string substitutions from the tarball Ben Hutchings <ben@decadent.org.uk> no 2018-08-10
206_translate-docs.diff Support translated docs using po4a Ben Hutchings <ben@decadent.org.uk> no debian 2022-08-01
0003-Add-German-translation-of-documentation.patch Add German translation of documentation
This was done by Helge Kreutzmann.
Ben Hutchings <ben@decadent.org.uk> no 2022-08-01
201_make-more-docs.diff Add rules and script to build manual pages and HTML
Halibut already supports these formats but since the documentation is
all combined we need to do a bit more work to extract the right
information for each game's manual page.
Ben Hutchings <ben@decadent.org.uk> no 2022-08-01
202_online-help.diff Add HTML-based online help
This works along the same lines as the Windows implementation,
though we have to try a bit harder to find a help browser.
Ben Hutchings <ben@decadent.org.uk> no 2018-08-10
207_slant-shade-filled.diff slant: Shade filled squares Ben Hutchings <ben@decadent.org.uk> no debian 2018-08-10
303_show-debian-version-number.diff Show Debian package version number
Include Debian version number in any version display to make
it obvious that the binaries are built from modified source.
Ben Hutchings <ben@decadent.org.uk> no 2018-08-10
install-two-icon-sizes.patch Install both 48x48 and 96x96 icons
gnome-shell prefers to use 96x96 icons, so install them as well as the
48x48 icons. Install them in size-specific directories instead of
using suffixes to the filename, and change the desktop files
accordingly.
Ben Hutchings <benh@debian.org> no debian 2023-01-22
0001-Black-Box-reject-negative-ball-counts-in-game_params.patch [PATCH 001/159] Black Box: reject negative ball counts in game_params.

You can inject one via a game desc string such as "10x10M5m-1", and
it's clearly silly.

_Zero_ balls, on the other hand, are a perfectly fine number: there's
nothing incoherent about a BB puzzle in which the possible numbers of
balls vary from (say) 0 to 5 inclusive, so that part of the challenge
is to work out as efficiently as possible whether there are even any
balls at all.

(We only need to check minballs, because once we know minballs >= 0,
the subsequent check ensures that maxballs >= minballs, and hence, by
transitivity, maxballs >= 0 too.)
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=5cac6a09c4db2b7e05c3e8dfd8920e2cdd1b8b03 2023-01-22
0002-Add-validate_params-bounds-checks-in-a-few-more-game.patch [PATCH 002/159] Add validate_params bounds checks in a few more games.

Ben tells me that his recent work in this area was entirely driven by
managed to prove that the lack of them allowed something buggy to
happen.

It seemed worth doing an eyeball-review pass to complement that
strategy, so in this commit I've gone through and added a few more
checks that restrict the area of the grid to be less than INT_MAX.

Notable in this commit: cube.c had to do something complicated because
in the triangular-grid modes the area isn't calculated as easily as
w*h, and Range's existing check that w+h-1 < SCHAR_MAX is sufficient
to rule out w*h being overlarge _but_ should be done before w*h is
ever computed.
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=b907e278751b740da7b9dc00c0cbdb93e7498919 2023-01-22
0006-Don-t-allow-Bridges-games-with-2-islands.patch [PATCH 006/159] Don't allow Bridges games with < 2 islands
Technically, a game with no islands is always solved, but it causes a
null-pointer dereference at startup because there's nowhere to put the
cursor. Games with one island are always insoluble because the island
must have at least one bridge and there's nowhere for it to go. So
the minimum playable game has two islands.

To demonstrate the segfault, try loading this save file:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :7:Bridges
PARAMS :1:3
CPARAMS :1:3
DESC :1:i
NSTATES :1:1
STATEPOS:1:1
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=a98ac4bb428ab5c1ff665aa1def6cc14d02a4e19 2023-01-28
0041-Fix-memory-leaks-in-Keen-s-validate_desc.patch [PATCH 041/159] Fix memory leaks in Keen's validate_desc()
Keen uses a DSF to validate its game descriptions and almost always
failed to free it, even when the validation succeeded.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=bb31efdbc91495a4385ca0afffa4c8bb8f564d7b 2023-02-13
0007-Forbid-moves-that-fill-with-the-current-colour-in-Fl.patch [PATCH 007/159] Forbid moves that fill with the current colour in Flood

This avoids an assertion failure, "oldcolour != newcolour" in fill(),
by catching it it execute_move(). As far as I know this couldn't be
triggered from the UI, but it could be demonstrated with this save
file:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :1:3
CPARAMS :1:3
DESC :12:231353400,11
NSTATES :1:3
STATEPOS:1:3
MOVE :2:M3
MOVE :2:M3
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=eb1ae3f3d041f9ff0c11b04613a695be11bda706 2023-01-28
0008-Cleanly-reject-ill-formed-solve-moves-in-Flood.patch [PATCH 008/159] Cleanly reject ill-formed solve moves in Flood
A solve move containing characters other than digits and commas would
cause an assertion failure, "*p == ','", in execute_move(). Such a move
can't as far as I know be generated in play, but can be read from a
corrupt save file.

Here's a sample of such a save file:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :7:3x3c6m5
CPARAMS :7:3x3c6m5
DESC :12:403011503,10
NSTATES :1:2
STATEPOS:1:2
SOLVE :2:SA
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e4112b322e299a461ddc46daee741c73733e186d 2023-01-28
0009-Don-t-segfault-on-premature-solve-moves-in-Mines.patch [PATCH 009/159] Don't segfault on premature solve moves in Mines
If a save file contained a solve move as the first move, Mines would
dereference a null pointer trying to look up the (at that point
undetermined) mine locations. Now execute_move() politely returns
NULL instead.

This save file demonstrates the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Mines
PARAMS :5:3x3n0
CPARAMS :5:3x3n0
DESC :127:r0,u,7a142789cabddc3fc4dcb7d2baa4a4937b33c9613ea870ac098e217981ad339930af585557d62048ea745d05b01475d9699596b394cc0adeebf0440a02
UI :2:D0
TIME :1:0
NSTATES :1:2
STATEPOS:1:2
SOLVE :1:S
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=28671e76b736aeb860b1f725898c45fe70ae6212 2023-01-28
0010-Limit-number-of-mines-in-Mines-game-description.patch [PATCH 010/159] Limit number of mines in Mines game description
Without this, it's possible to specify a description that has more
mines than there are places on the board to place them, which
eventually leads to a division by zero. This can be demonstrated by
entering a game description of "3:r8,u," and then clicking anywhere on
the board.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=75e8a1a9cabe7567f6019b1226783b61ba1ac42f 2023-01-28
0011-Validate-the-number-of-pegs-and-holes-in-a-Pegs-game.patch [PATCH 011/159] Validate the number of pegs and holes in a Pegs game ID

Without this, "1:O" causes an assertion violation, '!"new_ui found
nowhere for cursor"'. We may as well require two pegs and one hole,
since that's the minimum for a game in which there are any moves to
make.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=4359f59dd22770a94e29b2ddd54b533ad1713550 2023-01-28
0017-Mines-forbid-moves-that-flag-or-unflag-an-exposed-sq.patch [PATCH 017/159] Mines: forbid moves that flag or unflag an exposed square

interpret_move() couldn't generate them, but execute_move() also needs
to forbid them to defend against corrupt save files. I don't think this
actually caused any crashes, but it did cause unexpected "1" squares not
adjacent to mines.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=17364455186ae61e015d0f0de3f09423f78d0727 2023-02-01
0019-Avoid-invalid-moves-when-solving-Tracks.patch [PATCH 019/159] Avoid invalid moves when solving Tracks
The solver, when it decided that an edge or square should be both TRACK
and NOTRACK, would correctly decide that the puzzle was insoluble, but
would also mark the edge with both flags in its working copy. This
could then lead to assertion violations if that working copy of the
board was used for something else, for instance if it was fed back into
the solver. This couldn't happen in normal play, since failed solutions
just cause the solve command to fail, but the diagnostic "H" command
could trigger it from a save file, causing an assertion failure:
"state->sflags[y*state->p.w + x] & S_CLUE".

Now when the solver runs into this situation, it marks the puzzle as
insoluble but doesn't set the invalid flag, so the board remains valid
and future solve operations are safe.

This save file is the one that demonstrated the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :12:Train Tracks
PARAMS :5:6x6t0
CPARAMS :5:6x6t0
DESC :31:b0t9l,,S0,00,0,0,4,0,0,S0,0,0,0
NSTATES :1:8
STATEPOS:1:2
MOVE :1:H
MOVE :1:H
MOVE :1:H
MOVE :1:H
MOVE :1:H
MOVE :1:H
MOVE :1:H
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=875f0af21fbced5cbf6cf63b86fe3dc51682c863 2023-02-01
0020-Fix-move-validation-in-Netslide.patch [PATCH 020/159] Fix move validation in Netslide
The maximum length of a column move in Netslide is the height of the
puzzle, and the maximum length of a row move is the width, not the
other way around.

Moves of absolute length more than 1 can't be generated by
interpret_move(), but they can come from save files. On non-square
grids, the incorrect check led to assertion failures: "0 <= tx && tx <
w" and "0 <= ty && ty < h". This save file demonstrates the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :8:Netslide
PARAMS :3:4x9
CPARAMS :3:4x9
DESC :39:0000000000000h0h0000000000000000000000v
NSTATES :1:2
STATEPOS:1:2
MOVE :4:R0,5
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=ed0e4c304bed990948541fc0cf87309d75653806 2023-02-01
0021-Tighten-validation-of-Tents-game-descriptions.patch [PATCH 021/159] Tighten validation of Tents game descriptions
Specifically, TENT and NONTENT markers ('!' and '-') cannot appear as
the first or last character of a description, because that would
attempt to place them on squares outside the grid. This was caught by
assertions in new_game(), as can be demonstrated by feeding these
descriptions to older versions of Tents: "4:-p,0,0,0,0,0,0,0,0"
("new_game: Assertion `i >= 0 && i <= w*h' failed.") and
4:p-,0,0,0,0,0,0,0,0 ("new_game: Assertion `*desc == ','' failed.").
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=ed682bd5c608156d12ebaa2d84c4ce2e2877c10a 2023-02-02
0022-Dominosa-require-the-two-halves-of-a-domino-to-be-ad.patch [PATCH 022/159] Dominosa: require the two halves of a domino to be adjacent

Also that a line indicating no domino be between adjacent squares.
Without this, execute_move would allow you to place dominos and edges
between any pair ot squares, and then generate assertion failures
("execute_move: Assertion `d2 - w >= 0' failed." and "execute_move:
Assertion `d1 - w >= 0' failed.") when a domino was placed over an
invalid edge. This example save file demonstrates the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :8:Dominosa
PARAMS :1:6
CPARAMS :1:6
DESC :56:55521461210004364611033535444421636022603153156422620503
NSTATES :1:3
STATEPOS:1:3
MOVE :4:E0,2
MOVE :4:D0,2
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=294a3ac6e703c2820ceb7b28a1a5492b61e9a531 2023-02-02
0023-Forbid-lines-off-the-grid-in-Pearl.patch [PATCH 023/159] Forbid lines off the grid in Pearl
While they couldn't be generated in normal play, execute_move() would
permit lines and marks across the edge of the grid that would then
generate assertion failures ("dsf_update_completion: Assertion
`INGRID(state, bx, by)' failed.").

I've added a check to execute_move() that after updating a square, the
square doesn't have any lines or marks that leave the grid.

This save file demonstrated the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSZON :1:1
GAME :5:Pearl
PARAMS :5:5x6dt
CPARAMS :5:5x6dt
DESC :6:eeeeee
NSTATES :1:2
STATEPOS:1:1
MOVE :6:F1,4,2
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=15f4fa851a5781cf77984a6046405ffa758e7b33 2023-02-02
0024-Tolerate-incorrect-solutions-in-Inertia.patch [PATCH 024/159] Tolerate incorrect solutions in Inertia
The "solve" operation in Inertia generates a proposed solution as a
move string. But if such a move string is loaded from a save file it
might not actually describe a solution. If that happens then it's
possible to reach the end of the "solution" without winning, and doing
so should probably cause a recalculation of the solution rather than
an assertion failure ("execute_move: Assertion `ret->solnpos <
ret->soln->len' failed.").

I am a little concerned by the way that normal solve operations end up
encoded in the save file, but the re-solvings caused by going off
course don't, but I haven't got a good answer to that.

Here's a save file that demonstrates the assertion failure:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :7:Inertia
PARAMS :3:8x8
CPARAMS :3:8x8
DESC :64:sbgwsmswwgggwggmmbwgwbssbwbsbwbbwsSmwbbsbbmggbmssgmgwbmmwmbmmwsw
NSTATES :2:3
STATEPOS:1:1
MOVE 000:2:S0
MOVE 000:2:00
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=843d4ca17def11671809786f2a5aebd75f230dd9 2023-02-03
0025-Palisade-replace-dfs_dsf-with-a-simple-iteration.patch [PATCH 025/159] Palisade: replace dfs_dsf() with a simple iteration.
The whole purpose of a dsf is that you can traverse the edges of your
graph in any order you feel like. So if you want to build the
connected components of a graph you can just loop over all the edges
once. There's no need to run a depth-first search.

In fact there were an amazing number of things wrong with this 10-line
function:

- As Ben points out in commit 21193eaf9308ace, it didn't bother with
bounds checking when searching the grid, instead relying on the
never-removed grid boundary to stop the search - which was fragile in
the face of other bugs.

- The recursion uses linear stack, which is much worse than linear
heap, since stacks are often much more limited. (And the dsf _also_
used linear heap.)

- The recursion was completely unnecessary.

- The function used internal knowledge about dsf.c in order to define
the value UNVISITED to match what would happen to work.

- The name 'dfs_dsf' is totally confusing and almost impossible to
type!
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=517b14e666b0b71fc0bcd5da1b22cdc90d3434c9 2023-02-03
0026-latin_solver_alloc-handle-clashing-numbers-in-input-.patch [PATCH 026/159] latin_solver_alloc: handle clashing numbers in input grid.

In the setup phase of the centralised latin.c solver, we start by
going over the input grid containing already-placed clue numbers, and
calling latin_solver_place to enter each on into the solver's data
structure. This has the side effect of ruling out each number from the
rest of the row and column, and _also_ checking by assertion that the
number being placed is not ruled out.

Those are a bad combination, because it means that if you give an
obviously inconsistent input grid to latin_solver_alloc (e.g. with two
identical numbers in a row already), it will fail an assertion. In
that situation, you want the solver run as a whole to return
diff_impossible so that the error is reported cleanly.

This assertion failure could be provoked by giving either Towers or
Group a manually-constructed game description inconsistent in that
way, and hitting Solve. Worse, it could be provoked during live play
in Unequal, by filling in a number clashing with a clue and then
pressing 'h' to get hints.
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=5030d87903191d581586ecda2382ad5bcd70f63d 2023-02-05
0027-Pearl-fix-assertion-failure-on-bad-puzzle.patch [PATCH 027/159] Pearl: fix assertion failure on bad puzzle.
Similarly to the previous commit, if you started Pearl with at least
some kinds of invalid puzzle (e.g. "6x6:abBfWcWWrBa") and then pressed
'h' to get hints, you could provoke an assertion failure. But this
time the assertion wasn't in the solver itself; the solver gave up
gracefully and didn't crash, but it _did_ leave the links between
squares in the game_state in an inconsistent state, in that one square
was marked as linking to its neighbour without the neighbour also
linking back to it. This caused the /* should have reciprocal link */
assertion in dsf_update_completion to fail, when that was called from
check_completion after the solver had finished, to decide whether the
puzzle was now solved.

In this commit, I arrange that whether or not pearl_solve returns a
grid layout that's legal by the rules of the _puzzle_, it at least
returns one that's legal by the rules of the _data representation_, in
that every link between squares is either bidirectional or absent.

This is a better solution than just removing the assertion, because if
the inconsistent data were allowed to persist, it would lead to
further problems in gameplay. For example, if you just remove that
assertion instead of this fix and press 'h' on the example puzzle id
above, you'll find that the non-reciprocal links are actually visible,
in the form of several thick lines that stop at a grid square boundary
instead of connecting two square-centres. (It looks even sillier if
you set PEARL_GUI_LOOPY=y.)

That's a situation that can't be created by a normal move, and if you
try to make normal moves after it (e.g. click one of the weird edges),
you'll find that both sides of the half-link get toggled, so now it's
a half-link the other way round. So not only can't you _create_ this
situation in normal play, you can't get rid of it either!

That assertion in dsf_update_completion was commented out at one
point, and I put it back in commit c5500926bf7458a saying that if it
failed I'd like to know about it. And indeed, I'm glad I did, because
this kind of unfixable wrongness in the resulting game_state was worth
noticing and getting rid of!
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=05c536e50d0c3114e1de5283eac97ff22bad0fc7 2023-02-05
0028-Pearl-fix-bounds-check-in-previous-commit.patch [PATCH 028/159] Pearl: fix bounds check in previous commit.
Ahem. That's what I get for testing the fix on a square puzzle.
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=9ce0a6d93212ffc0986a2b399fd8e9e983f62904 2023-02-05
0042-Remember-to-free-the-actual_board-array-in-Mosaic.patch [PATCH 042/159] Remember to free the actual_board array in Mosaic Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=0f20b7226957a2fceff37a67b0d9874c7db3a7fc 2023-02-13
0029-Unequal-Don-t-insist-that-solve-moves-must-actually-.patch [PATCH 029/159] Unequal: Don't insist that solve moves must actually solve

A corrupt save file can include an "S" move that doesn't give a valid
solution. An assertion failure ("execute_move: Assertion `rc > 0'
failed.") at that point is rude, so now we just don't set the
"completed" flag in that case. We still set the "cheated" flag, to
reward (lack of) effort.

Here's a trivial test case:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :7:Unequal
CPARAMS :1:3
PARAMS :1:3
DESC :17:0,0,0,0,0,0,0,0,0
NSTATES :1:2
STATEPOS:1:2
MOVE :10:S222222222
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=84ec2a0a77d63450311f7c25b36d4b9f7e3c53e1 2023-02-04
0030-Range-Don-t-fail-an-assertion-on-an-all-black-board.patch [PATCH 030/159] Range: Don't fail an assertion on an all-black board
If there are no white squares, then Range's check that all the white
squares form a connected component goes wrong. Skip the check in that
case to avoid an assretion violation ("edsf_canonify: Assertion `index
>= 0' failed."). This can be demonstrated by starting a game with no
clues (e.g. "range 3:i") and then filling in every square.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=ae73ad76ef95f0e40868436cb750126322051dd0 2023-02-04
0031-Limit-width-and-height-to-SHRT_MAX-in-Mines.patch [PATCH 031/159] Limit width and height to SHRT_MAX in Mines
Mines' "struct set" stores co-ordinates within the grid in a pair of
shorts, which leads to very bad behaviour (including heap-based buffer
overruns) if the grid is bigger than SHRT_MAX in either dimension. So
now we don't allow that.

The overrun can be demonstrated by loading this save file, though the
precise crash is quite variable. In particular, you seem to get
better crashes if the file doesn't have a trailing newline.

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
PARAMS :5:06000
CPARAMS :7:6x60000
NSTATES :1:3
STATEPOS:1:2
MOVE :5:C0,00
GAME :5:Mines
DESC :22:r8,u,00000000000000000
MOVE ::
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=c0e08f308792b15425e10ad494263d77a45ad92d 2023-01-28
0032-Mines-Add-assertions-to-range-check-conversions-to-s.patch [PATCH 032/159] Mines: Add assertions to range-check conversions to short

I think these should be adequately guarded by the new restrictions on
grid size, but I'd prefer to be sure.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=49841bd0fc04490d94cf32c0e6f9d3f4ffabe098 2023-01-31
0033-Unequal-fix-sense-error-in-latin_solver_alloc-fix.patch [PATCH 033/159] Unequal: fix sense error in latin_solver_alloc fix.
In commit 5030d87903191d5 I gave latin_solver_alloc a return value,
and introduced a check of that value at every call site. One of the
checks was backwards, with the effect that Unequal game generation now
more or less always fails an assertion. For example:

$ unequal --generate 1 4#12345
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=bd5c0a37a019c540eda05f8291cad90ffd598134 2023-02-08
0034-Forbid-impossible-moves-in-Bridges.patch [PATCH 034/159] Forbid impossible moves in Bridges
Specifically, a bridge or a non-bridge must connect two islands that
differ in precisely one co-ordinate. Without this, a save file that
tries to connect or disconnect two non-orthogonal islands will cause
"island_join: Assertion `!"island_join: islands not orthogonal."'
failed."

Here's a save file demonstrating the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :7:Bridges
PARAMS :13:3x3i30e10m2d0
CPARAMS :13:3x3i30e10m2d0
DESC :6:b1c1a2
NSTATES :1:2
STATEPOS:1:2
MOVE :10:L0,2,2,0,1
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=bf9abb2a127a4a81babe50ecb419527f8aeffe28 2023-02-10
0035-Forbid-game-descriptions-with-joined-islands-in-Brid.patch [PATCH 035/159] Forbid game descriptions with joined islands in Bridges

A game description with islands in adjacent grid squares, like
"3x3:11g", shouldn't be allowed. If it is, then bridges between the
islands are invisible and clicking one of them causes an assertion

The code to check this is really rather complex, but I think the
complexity is mostly necessary.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=ad2fb760fc881d1ceb1ac1151bf60b85deb16c71 2023-02-10
0037-Check-state-is-valid-at-the-end-of-a-move-in-Pearl.patch [PATCH 037/159] Check state is valid at the end of a move in Pearl
A Pearl move string contains a sequence of sub-moves, each of which
can affect the state of the connection between the centre of a square
and one of its edges. interpret_move() generates these in pairs so
that the two halves of a connection between the centres of adjacent
squares stay in the same state.

If, however, a save file contains mismatched half-moves,
execute_move() should ideally return NULL rather than causing an
assertion failure. This has to be checked at the end of the whole
move string, so I've arranged for check_completion() to return a
boolean indicating whether the current state (and hence the move
preceding it) is valid. It now returns 'false' when a connection
stops at a square boundary or when it goes off the board. These
conditions used to be assertion failures, and now they just cause the
move to be rejected.

This supersedes the check for off-board connections added in 15f4fa8,
since now check_completion() can check for off-board links for the
whole board at once.

This save file trivially demonstrates the problem, causing
"dsf_update_completion: Assertion `state->lines[bc] & F(dir)' failed"
without this fix:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :5:Pearl
PARAMS :5:6x6t0
CPARAMS :5:6x6t0
DESC :17:BbBfWceBbWaBWWgWB
NSTATES :1:2
STATEPOS:1:2
MOVE :6:R1,0,0
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=c0b2f0fc98e87392dcb4dd8faf3076786fc49367 2023-02-11
0038-Cleanly-reject-more-ill-formed-solve-moves-in-Flood.patch [PATCH 038/159] Cleanly reject more ill-formed solve moves in Flood
The fix in e4112b3 was incomplete: there was another assertion that could be failed by a save file with an ill-formed solve move. That now gets rejected properly. Here's an example save file to demonstrate the problem:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :5:Flood
PARAMS :7:6x6c6m0
CPARAMS :7:6x6c6m0
DESC :39:000000000000000000000000000000000000,00
NSTATES :1:2
STATEPOS:1:2
MOVE :1:S
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=896a73bd7ff8cbde44e97d89cef57346478f0072 2023-02-11
0039-Don-t-allow-moves-that-change-the-constraints-in-Une.patch [PATCH 039/159] Don't allow moves that change the constraints in Unequal

Unequal has a flags word per cell. Some of those flags are fixed,
like the locations of the ">" signs, but others indicate errors and
are used to allow the player to mark clues as "spent". Move strings
beginning with "F" allow the user to change the "spent" flags, but
they shouldn't allow the user to change any other flags, especially
those marking the constraints.

Without this fix, the following save file gives a "solver_nminmax:
Assertion `x >= 0 && y >= 0 && x < o && y < o' failed" after it adds a
clue that points off the board:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
GAME :7:Unequal
PARAMS :3:3e0
CPARAMS :3:3e0
DESC :17:0,0,0,0,0,0,0,0,0
NSTATES :2:3
STATEPOS:1:3
MOVE :6:F2,0,4
MOVE :1:H
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=97b03cc67a31c1d0869a21c50b9ca31f78775ff9 2023-02-11
0045-Undead-check-the-return-value-of-sscanf-in-execute_m.patch [PATCH 045/159] Undead: check the return value of sscanf() in execute_move()

sscanf() assigns its output in order, so if a conversion specifier fails
to match, a later "%n" specifier will also not get its result assigned.
In Undead's execute_move(), this led to the result of "%n" being used
without being initialised. That could cause it to try to parse
arbitrary memory as part of the move string, which shouldn't be a
security problem (since execute_move() handles untrusted input anyway),
but could lead to a crash and certainly wasn't helpful.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=0a7c531e8f4c1970662f7c30aea006e65d5ff010 2023-02-13
0046-Don-t-leak-duplicate-edges-in-Untangle.patch [PATCH 046/159] Don't leak duplicate edges in Untangle
Untangle game descriptions are allowed to contain duplicate edges, and
add234() can handle deduping them. However, when add234() reports that
your newly-allocated edge is a duplicate, it's important to free it
again.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=19401e95e0a75577103e9c1a877611234a0d8ab5 2023-02-13
0047-Remember-to-free-the-numcolours-array-from-Pattern-s.patch [PATCH 047/159] Remember to free the numcolours array from Pattern's drawstate Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=1aa67e7a75ac21d15780d6aaab65a2c0f6f65198 2023-02-13
0048-Free-new-game_state-properly-in-Mosaic-s-execute_mov.patch [PATCH 048/159] Free new game_state properly in Mosaic's execute_move()

Using sfree() rather than free_game() in the error paths meant that
various arrays referenced from the game_state weren't properly freed.
Also one error path didn't free the game_state at all.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=d577aaecab09506988a657fa257c4d0ab85d0cd6 2023-02-13
0049-Twiddle-don-t-read-off-the-end-of-parameter-strings-.patch [PATCH 049/159] Twiddle: don't read off the end of parameter strings ending 'm'

The overrun could be demonstrated by specifying a parameter string of
"3x3m" to a build with AddressSanitizer.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=73c7bc090155ab8c4661feaeea9e6a6e74ee6f77 2023-02-13
0050-Loopy-free-the-grid-description-string-if-it-s-inval.patch [PATCH 050/159] Loopy: free the grid description string if it's invalid Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e336513be755159158c5ba017c91b018ad4cd36c 2023-02-13
0051-Mosaic-don-t-duplicate-the-description-being-validat.patch [PATCH 051/159] Mosaic: don't duplicate the description being validated

Mosaic's validate_desc() doesn't write to the description string, so
it has no need to make a copy of it. And if it doesn't copy it, it
can't leak the copy.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=da2767a3f9bf4abb0436157972366202ad53a407 2023-02-13
0055-Validate-that-save-file-values-are-ASCII-mostly.patch [PATCH 055/159] Validate that save file values are ASCII (mostly)
Apart from "SEED" records, all values in save files generated by Puzzles
should be printable ASCII. This is enforced by assertion in the saving
code. However, if a save file with non-ASCII move strings (for
instance) manages to get loaded then these non-ASCII values can cause an
assertion failure on saving. Instead, the loading code now checks
values for ASCIIness.

This will not only avoid problems when re-saving files, but will also
defend the various internal parsers from at least some evil strings. It
shouldn't invalidate any save files actually generated by Puzzles, but
it will sadly invalidate some of my fuzzing corpus.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=c3a5a7842eb6c41fb75a8a110a3f2cbc1c8fc5d9 2023-02-12
0056-More-validation-of-solve-moves-in-Flood.patch [PATCH 056/159] More validation of solve moves in Flood
To avoid assertion failures while painting it, we need to ensure that
the purported solution in a solve move doesn't include filling with the
current top-left colour at any point. That means checking the first
entry against the current top-left colours, and each later one against
its predecessor.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e8668dc883e940f0852ff4520abc3d30cae90aef 2023-02-13
0058-Make-sure-that-moves-in-Flood-use-only-valid-colours.patch [PATCH 058/159] Make sure that moves in Flood use only valid colours
If execute_move() receieves a move that uses a colour beyond the range
for the current game, it now rejects it. Without this a solve string
containing an invalid colour would cause an assertion failure: "fill:
Assertion `oldcolour != newcolour' failed." While I was in the area I
put a range check on colours for normal moves as well. To demonstrate
the problem, load this save file:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Flood
PARAMS :7:6x6c6m5
CPARAMS :7:6x6c6m3
DESC :39:432242034203340350204502505323231342,17
NSTATES :1:2
STATEPOS:1:2
MOVE :2:S6
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=7364ce8e266d947be146d635958a7b282752aac6 2023-02-14
0059-Tighten-grid-size-limit-in-Mines.patch [PATCH 059/159] Tighten grid-size limit in Mines
Mines uses random_upto() to decide where to place mines, and
random_upto() takes a maximum limit of 2^28-1, so limit the number of
grid squares to that (or INT_MAX if someone's still trying to build on
a 16-bit system).

This avoids an assertion failure: "random_upto: Assertion `bits < 32'
failed." which can be demonstrated by this save file:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :5:Mines
PARAMS :5:18090
CPARAMS :5:18090
DESC :11:r9,u,MEdff6
UI :2:D0
TIME :1:0
NSTATES :1:2
STATEPOS:1:2
MOVE :4:O2,1
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=9394e9c74bdb48dc1c74693bcb41fd35f8fc743c 2023-02-15
0102-Map-reduce-maximum-size.patch [PATCH 102/159] Map: reduce maximum size
validate_desc relies on being able to calculate 2*wh in an int, so the
maximum grid size is at most INT_MAX/2.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e2d390aae872cee4cb16d746af3b2eeb7713cbf5 2023-02-26
0061-Solo-cope-with-pencil-marks-when-tilesize-1.patch [PATCH 061/159] Solo: cope with pencil marks when tilesize == 1
Solo's layout calculations for pencil marks could fail with a tilesize
of 1, generating an assertion failure: "draw_number: Assertion `pbest
> 0' failed." This was reported as Debian bug #905852.

My solution is slightly silly, namely to change a ">" in the test for
whether a new layout is the best so far to ">=". This allows for
finding a (terrible) layout even for tilesize == 1, and also has the
side-effect of slightly preserring wide layouts over tall ones.
Personally, I think that's an improvement.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=3cd51d001769c657ebb4184bd05343af4d7e12b1 2023-02-16
0065-Tracks-set-drag_s-x-y-even-if-starting-off-grid.patch [PATCH 065/159] Tracks: set drag_s{x,y} even if starting off-grid
Otherwise, if subsequent mouse/finger movement lines up with the previous
drag attempt's start, then suddenly a drag is in progress from there, which
is confusing.

Fixes #588

(cherry picked from Android port,
commit 8ce1bbe460d70a915caf2dbeb30354d22dc8a8ef)
Chris Boyle <chris@boyle.name> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=111db0743bd273c340dd683449076fd9ec5797f5 2023-02-10
0080-Undead-be-a-bit-more-careful-about-sprintf-buffer-si.patch [PATCH 080/159] Undead: be a bit more careful about sprintf buffer sizes Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=448095ede815b1a63ddedc602c3ac768a0d52968 2023-02-18
0090-Fix-memory-leak-in-midend_game_id_int.patch [PATCH 090/159] Fix memory leak in midend_game_id_int()
The "par" string wasn't getting freed on some error paths. Fixed by
freeing it immediately after its last use, which is before any of the
error paths.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=4e09175fdaaffc0483fc9bf767311268794d956c 2023-02-20
0092-Flood-don-t-read-off-the-end-of-some-parameter-strin.patch [PATCH 092/159] Flood: don't read off the end of some parameter strings

This is essentially the same fix as 73c7bc090155ab8c was for Twiddle.
The new code is less clever but more correct (and more obviously
correct). The bug could be demonstrated by using a parameter string
of "c" or "m" with an AddressSanitizer build of Flood.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=bbe866a3819c6a754a5b1d8c5bc5d0701796acfb 2023-02-20
0101-Be-more-careful-with-type-of-left-operand-of.patch [PATCH 101/159] Be more careful with type of left operand of <<
On a 32-bit system, evaluating 1<<31 causes undefined behaviour because
1 is signed and so it produces signed overflow. UBSan has spotted a
couple of occasions where this happens in Puzzles, so in each case I've
converted the left operand to the unsigned result type we actually want.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=93be3f7ccaa63b0fd953bcfd88d685b47b76605e 2023-02-26
0103-Correctly-handle-some-short-save-files.patch [PATCH 103/159] Correctly handle some short save files
A save file that ended in the middle of a value before the "SAVEFILE"
field had been loaded would cause a read from uninitialised memory.
While technically undefined behaviour this was practically pretty
harmless. Fixed by handling unexpected EOF here the same an
unexpected EOF anywhere else.

This bug could be demonstrated by loading a truncated save file like
this in a build with MemorySanitizer enabled:

SAVEFILE:41:Simo
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=6ee62a43abe7d7e77226415b21d1cbf16dbda85a 2023-02-26
0104-Inertia-insist-that-solutions-must-be-non-empty.patch [PATCH 104/159] Inertia: insist that solutions must be non-empty
Any solution actually generated by the solver will contain at least one
move, because it refuses to solve games that are already solved.
However, a save file might contain an empty "solve" move. This causes
an uninitialised read when execute_move() then tries to check if the
next move is in accordance with the solution, because the check for
running off the end of the solution happens after that.

We now avoid this by treating a zero-length "solution" as an invalid
move.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=5a491c5ad333ef34c1e7713f920f51cbb205af60 2023-02-26
0115-Galaxies-fix-recursion-depth-limit-in-solver.patch [PATCH 115/159] Galaxies: fix recursion depth limit in solver.
The static variable 'solver_recurse_depth' is _mostly_ used by the
standalone solver, to appropriately indent the solver diagnostics for
the current recursion level. So most uses of it are guarded by an
'#ifdef STANDALONE_SOLVER' statement, or some equivalent (such as
being inside the solvep() macro).

One exception is the check that limits the recursion depth to 5, to
avoid getting hung up forever on a too-hard game. Unfortunately, this
check depends on the variable actually incrementing when we recurse
another level - and it wasn't, because the increment itself was under
ifdef! So the generator in live Galaxies could recurse arbitrarily
deep, and generate puzzles that the standalone solver found too hard
_even_ at Unreasonable mode.

Removed the ifdefs, so that solver_recurse_depth is now incremented
and decremented. Also, make sure to initialise the depth to 0 at the
start of a solver run, just in case it had a bogus value left over
from a previous run.
Simon Tatham <anakin@pobox.com> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=f018ef97d34b9126744e63cc2112c302fcae4ab4 2023-03-12
0138-Correct-a-range-check-in-Magnets-layout-verification.patch [PATCH 138/159] Correct a range check in Magnets' layout verification
Squares in the grid are numbered from 0, so the upper limit check
needs to use "<=" rather than "<". Without this, invalid descriptions
can cause a read overrun off the end of the board.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=91735e5019be84d2fa693c5d40746c818ace28f8 2023-03-31
0139-Magnets-add-a-check-that-magnets-don-t-wrap-between-.patch [PATCH 139/159] Magnets: add a check that magnets don't wrap between lines

There was nothing in Magnet's description validation to prevent there
being the left end of a magnet at the right end of a row and the right
end of a magnet at the left end of the row below. Indeed as far as I
can such a game (e.g. 3x3:..2,2..,...,1.1,TLRB*LRLR) plays entirely
correctly except that one magnet is discontinuous.

While this worked, it was entirely an artefact of the particular memory
layout that Magnets uses and shouldn't have been allowed, so I've added
an additional validation rule to stop it.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=0bd1a8057841386754f9f4a8a268616c7ce80e80 2023-04-01
0155-Net-assert-that-cx-and-cy-are-in-range-in-compute_ac.patch [PATCH 155/159] Net: assert that cx and cy are in range in compute_active()

This avoids an out-of-range heap write shortly afterwards. An assertion
failure is better than a buffer overrun, but still not ideal. Fixing
the problem properly will require fairly wide-ranging changes, though.

The bug can be demonstrated by loading this save file into a build with
AddressSanitizer:

SAVEFILE:41:Simon Tatham's Portable Puzzle Collection
VERSION :1:1
GAME :3:Net
PARAMS :4:5x5w
CPARAMS :4:5x5w
DESC :25:9893e85285bb72e6de5182741
UI :9:O0,0;C6,6
NSTATES :1:1
STATEPOS:1:1
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e411db788cfc0d0ed54b3c9b9deb15edba7d237a 2023-02-13
0159-Don-t-allow-zero-clues-in-Pattern.patch [PATCH 159/159] Don't allow zero clues in Pattern
Some nonogram implementations allow zero clues so that a row or column
with a single zero clue is equivalent to one with no clues, that is it
has no black squares in it. Pattern, however, doesn't interpret them
like this and treats a puzzle with a zero clue as insoluble, so it's
not helpful to permit them.

Permitting zero clues also confuses Pattern's memory allocation so
that it can suffer a buffer overrun. As an example, before this
commit a build with AddressSanitizer would report a buffer overrun
with the description "1:0/0.0" because it tries to put two clues in a
row that can have a maximum of one.
Ben Harris <bjh21@bjh21.me.uk> no debian https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=71cf891fdc3ab237ecf0e5d1aae39b6c9fe97a4d 2023-02-14

All known versions for source package 'sgt-puzzles'

Links