Validate untrusted input #49
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will allow Proxy.t to appear in the connection record.
Wayland requires that badly behaved clients be disconnected with a fatal protocol error. Implement this.
These functions prevent the common mistake of sending an invalid error code.
This logs all protocol errors sent at "warn" level.
No other change.
A proxy may choose to then send the protocol error to its own client.
These will be used to proxy errors sent by the host compositor.
Bad data should cause protocol errors to be raised, not assertion failures or Invalid_argument exceptions.
Wayland does not allow message lengths that exceed 4096, are less than 8, or are not a multiple of 4. Currently, libwayland always enforces that messages are at least 8 bytes and hangs the connection if messages exceed the buffer size (4096 bytes by default on server, unlimited on client). Enforce the stricter rules that will (hopefully) be enforced by future versions of libwayland. See https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/416.
libwayland's API is incapable of handling them, and starting with libwayland commit 6c4a695045155583a99f3fbce7bb745f79c2e726 they are rejected entirely. This commit will be included in libwayland 1.24 and later.
Some Wayland protocol implementations enforce that messages must be as small as possible, and libwayland will soon be one of these implementations. Therefore, correctly sizing serialized messages is mandatory. Unfortunately, the code did not count integer arguments correctly: it allocated space for every argument, rather than only integer arguments. Therefore, messages with non-integer arguments were over-allocated by 4 bytes for every non-integer arguemnt in the message. To ensure that the test suite catches bugs like this, reject trailing junk in ocaml-wayland too. Historically, the check for trailing junk was added to ocaml-wayland first. When it was found to break the test suite, the serialization bug was discovered and fixed. See libwayland MR https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/416.
ocaml-wayland can handle almost any Wayland message imaginable, so long as message sizes do not exceed 2**16 - 1. However, libwayland is much more limited: it cannot handle messages with more than 20 arguments or exceeding 4096 bytes. To protect code using libwayland, ensure that ocaml-wayland only generates messages that libwayland can handle. In libwayland 1.23, messages exceeding 4096 bytes put servers into a loop that consumes CPU. libwayland 1.22 and before disconnect clients with an unhelpful protocol error. I have filed an MR [1] that fixes the libwayland regression. Messages with more than 20 arguments result in errors from libwayland. [1]: https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/409
This is a protocol requirement since Wayland commit 2bbd80c8df129ee6810f6ec6d8d379ed3190760b. https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/422 adds the same check to libwayland.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on #48.