Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the eldbus library with gdbus for D-Bus communication within the accessibility channel, updating build configurations and refactoring the C++ implementation. However, it introduces critical and high-severity command injection vulnerabilities. A critical issue exists in tools/generate_sysroot.py due to improper handling of filenames in shell commands, and a high-severity vulnerability is present in flutter/shell/platform/tizen/channels/accessibility_channel.cc where untrusted input from the Flutter engine is used in a D-Bus method call without sufficient sanitization. Additionally, the new gdbus code could be improved by leveraging g_autoptr for automatic resource management, which is a GLib best practice that will enhance code safety and maintainability.
| eldbus_connection_send(accessibility_bus_, eldbus_message, nullptr, | ||
| nullptr, -1); | ||
| GVariant* params = g_variant_new("(sb)", msg->c_str(), TRUE); | ||
| g_dbus_connection_call( |
There was a problem hiding this comment.
Is this g_dbus_connection_call() working properly? When I tested it, the screen reader didn't read SemanticsService.announce() . (TV)
SemanticsService.announce("TEST", TextDirection.ltr);There was a problem hiding this comment.
The screen reader failed because the connection was closing. This was due to a missing G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION flag in the g_dbus_connection_new_for_address_sync() call, which is required for the standard handshake protocol of A11Y service. I've updated the code to include this flag.
| return; | ||
| } | ||
|
|
||
| g_dbus_proxy_call(bus_, "GetAddress", nullptr, G_DBUS_CALL_FLAGS_NONE, -1, |
There was a problem hiding this comment.
What happens if g_dbus_proxy_call() or the g_dbus_connection_call() function below fails?
I'm not sure, but it seems like error handling would be necessary.
There was a problem hiding this comment.
g_dbus_connection_call() is an asynchronous function. I have implemented the callback function where I handle error checks for the returned results: OnAccessibilityBusAddressGet() for GetAddress and _readCommandCallback() for ReadCommand.
4 participants
Fix #148