Skip to content

Commit

Permalink
Error when XBoard opponent has newline in name
Browse files Browse the repository at this point in the history
Use the configuration machinery to send the opponent's information
to the engine instead of using the Opponent class directly. This will
make sure that an exception is raised if the opponent's name has
newlines in it before send_line() is called. This prevents opponents
from injecting commands into the engine. For example, if an
opponent sets their name as "Cheat\neasy", then the following
commands will be sent to the local engine

name Cheat
easy

which would cause the engine to turn off pondering.

Also, add tests to show the change works as expected.
  • Loading branch information
MarkZH committed Jul 13, 2023
1 parent 77bb7b2 commit ee11a56
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
14 changes: 8 additions & 6 deletions chess/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -2152,6 +2152,7 @@ def _variant(self, variant: Optional[str]) -> None:

def _new(self, board: chess.Board, game: object, options: ConfigMapping, opponent: Optional[Opponent] = None) -> None:
self._configure(options)
self._configure(self._opponent_configuration(opponent=opponent))

# Set up starting position.
root = board.root()
Expand All @@ -2172,15 +2173,16 @@ def _new(self, board: chess.Board, game: object, options: ConfigMapping, opponen
if self.config.get("random"):
self.send_line("random")

opponent_name = (opponent.name if opponent else None) or self.target_config.get("name")
opponent_name = self.config.get("name")
if opponent_name and self.features.get("name", True):
self.send_line(f"name {opponent_name}")

opponent_rating = (opponent.rating if opponent else None) or self.target_config.get("opponent_rating") or 0
if self.target_config.get("engine_rating") or opponent_rating:
self.send_line(f"rating {self.target_config.get('engine_rating') or 0} {opponent_rating}")
opponent_rating = self.config.get("opponent_rating")
engine_rating = self.config.get("engine_rating")
if engine_rating or opponent_rating:
self.send_line(f"rating {engine_rating or 0} {opponent_rating or 0}")

if (opponent and opponent.is_engine) or (self.target_config.get("computer") if self.config.get("computer") is None else self.config.get("computer")):
if self.config.get("computer"):
self.send_line("computer")

self.send_line("force")
Expand Down Expand Up @@ -2518,7 +2520,7 @@ def _opponent_configuration(self, *, opponent: Optional[Opponent] = None, engine
if opponent is None:
return {}

opponent_info: Dict[str, Union[int, bool, str]] = {"engine_rating": engine_rating or 0,
opponent_info: Dict[str, Union[int, bool, str]] = {"engine_rating": engine_rating or self.target_config.get("engine_rating") or 0,
"opponent_rating": opponent.rating or 0,
"computer": opponent.is_engine or False}

Expand Down
9 changes: 9 additions & 0 deletions test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3688,6 +3688,15 @@ async def main():
self.assertEqual(result.move, board.parse_san("e5"))
mock.assert_done()

bad_opponent = chess.engine.Opponent("New\nLine", "GM", 1, False)
with self.assertRaises(chess.engine.EngineError):
await protocol.send_opponent_information(opponent=bad_opponent)
mock.assert_done()

with self.assertRaises(chess.engine.EngineError):
result = await protocol.play(board, limit, game="bad game", opponent=bad_opponent)
mock.assert_done()

asyncio.set_event_loop_policy(chess.engine.EventLoopPolicy())
asyncio.run(main())

Expand Down

0 comments on commit ee11a56

Please sign in to comment.