Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when XBoard opponent has newline in name #1021

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

MarkZH
Copy link
Contributor

@MarkZH MarkZH commented Jul 13, 2023

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.

Fixes #1018

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.
@MarkZH
Copy link
Contributor Author

MarkZH commented Jul 13, 2023

In the comment for the play() method, it says that if the options parameter is used to change the configuration of the engine, the original configuration would be restored after the move. Where does that happen? I want to make sure that the opponent information is restored, as well.

@niklasf
Copy link
Owner

niklasf commented Jul 13, 2023

The comment is not entirely true, and should maybe be corrected/clarified: The engine wrapper behaves as if it were true, but the restore does not happen immediately after play(). Instead, on the next call to play() the current configuration will be compared to the target configuration again. If any option is not given again, then, it would be restored at that point.

@niklasf niklasf merged commit fa152ad into niklasf:master Jul 13, 2023
8 of 18 checks passed
@MarkZH MarkZH deleted the send-line-security branch July 13, 2023 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review send_opponent_information() for UCI/XBoard command injection
2 participants