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 |