From 587cd0bf19337213f5dcafafa30ef85fe3106bdb Mon Sep 17 00:00:00 2001 From: Dor-bl <59066376+Dor-bl@users.noreply.github.com> Date: Sun, 5 Jul 2026 20:54:12 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=94=92=20Fix=20potential=20XPath=20in?= =?UTF-8?q?jection=20in=20=5Felement=5Ffind=5Fby=5Ftext?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The input string 'text' was directly concatenated into the XPath query using format(), which could allow a malicious user to manipulate the XPath structure. This is fixed by using the utility function escape_xpath_value() and updating the XPath templates to work with the escaped (and already quoted) values. PR Title: 🔒 [security fix potential XPath injection in _element_find_by_text] 🎯 What: Fixed potential XPath injection vulnerability in the `_element_find_by_text` method. ⚠️ Risk: Malicious input could be used to manipulate XPath queries, potentially allowing unauthorized access to elements or data within the application. 🛡️ Solution: Applied `utils.escape_xpath_value()` to the `text` parameter before incorporating it into XPath strings. Refactored the XPath templates to remove hardcoded quotes. Added comprehensive unit tests in `tests/keywords/test_element_security.py` to verify the fix. --- AppiumLibrary/keywords/_element.py | 11 +++-- tests/keywords/test_element_security.py | 64 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/keywords/test_element_security.py diff --git a/AppiumLibrary/keywords/_element.py b/AppiumLibrary/keywords/_element.py index f627ade..58520a3 100644 --- a/AppiumLibrary/keywords/_element.py +++ b/AppiumLibrary/keywords/_element.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- +from AppiumLibrary import utils from AppiumLibrary.locators import ElementFinder from appium.webdriver.common.appiumby import AppiumBy from .keywordgroup import KeywordGroup @@ -724,16 +725,18 @@ def _element_find_by_text(self, text, exact_match=False): if element: return element else: + _text = utils.escape_xpath_value(text) if exact_match: - _xpath = u'//*[@value="{}" or @label="{}"]'.format(text, text) + _xpath = u'//*[@value={text} or @label={text}]'.format(text=_text) else: - _xpath = u'//*[contains(@label,"{}") or contains(@value, "{}")]'.format(text, text) + _xpath = u'//*[contains(@label,{text}) or contains(@value, {text})]'.format(text=_text) return self._element_find(_xpath, True, True) elif self._get_platform() == 'android': + _text = utils.escape_xpath_value(text) if exact_match: - _xpath = u'//*[@{}="{}"]'.format('text', text) + _xpath = u'//*[@{attr}={text}]'.format(attr='text', text=_text) else: - _xpath = u'//*[contains(@{},"{}")]'.format('text', text) + _xpath = u'//*[contains(@{attr},{text})]'.format(attr='text', text=_text) return self._element_find(_xpath, True, True) def _get_text(self, locator, first_only: bool = True): diff --git a/tests/keywords/test_element_security.py b/tests/keywords/test_element_security.py new file mode 100644 index 0000000..c0d9f34 --- /dev/null +++ b/tests/keywords/test_element_security.py @@ -0,0 +1,64 @@ +import unittest +try: + from unittest import mock +except ImportError: + import mock +from AppiumLibrary.keywords._element import _ElementKeywords +from AppiumLibrary import utils +from appium.webdriver.common.appiumby import AppiumBy + +class ElementSecurityTests(unittest.TestCase): + def setUp(self): + self.element_keywords = _ElementKeywords() + self.driver = mock.Mock() + self.element_keywords._current_application = mock.Mock(return_value=self.driver) + # Mocking _info to avoid issues with BuiltIn + self.element_keywords._info = mock.Mock() + + def test_android_exact_match_secure(self): + self.element_keywords._get_platform = mock.Mock(return_value='android') + self.element_keywords._element_find = mock.Mock(return_value=mock.Mock()) + malicious_text = '"] | //*[@text="injected' + + self.element_keywords._element_find_by_text(malicious_text, exact_match=True) + + escaped_text = utils.escape_xpath_value(malicious_text) + # Now it should be //*[@text=concat('"', "'] | //*[@text=", '"', "injected")] + expected_xpath = u'//*[@text={}]'.format(escaped_text) + self.element_keywords._element_find.assert_any_call(expected_xpath, True, True) + + def test_android_contains_match_secure(self): + self.element_keywords._get_platform = mock.Mock(return_value='android') + self.element_keywords._element_find = mock.Mock(return_value=mock.Mock()) + malicious_text = '")] | //*[@text="injected' + + self.element_keywords._element_find_by_text(malicious_text, exact_match=False) + + escaped_text = utils.escape_xpath_value(malicious_text) + expected_xpath = u'//*[contains(@text,{})]'.format(escaped_text) + self.element_keywords._element_find.assert_any_call(expected_xpath, True, True) + + def test_ios_exact_match_secure(self): + self.element_keywords._get_platform = mock.Mock(return_value='ios') + # ios first tries to find by text directly (not via xpath) + self.element_keywords._element_find = mock.Mock(side_effect=[None, mock.Mock()]) + + malicious_text = '"] | //*[@label="injected' + + self.element_keywords._element_find_by_text(malicious_text, exact_match=True) + + escaped_text = utils.escape_xpath_value(malicious_text) + expected_xpath = u'//*[@value={text} or @label={text}]'.format(text=escaped_text) + self.element_keywords._element_find.assert_any_call(expected_xpath, True, True) + + def test_ios_contains_match_secure(self): + self.element_keywords._get_platform = mock.Mock(return_value='ios') + self.element_keywords._element_find = mock.Mock(side_effect=[None, mock.Mock()]) + + malicious_text = '")] | //*[@label="injected' + + self.element_keywords._element_find_by_text(malicious_text, exact_match=False) + + escaped_text = utils.escape_xpath_value(malicious_text) + expected_xpath = u'//*[contains(@label,{text}) or contains(@value, {text})]'.format(text=escaped_text) + self.element_keywords._element_find.assert_any_call(expected_xpath, True, True) From 0d4129b613ff230697789c21ff3ba268807ce3e6 Mon Sep 17 00:00:00 2001 From: Dor-bl Date: Fri, 10 Jul 2026 23:42:32 +0200 Subject: [PATCH 2/2] test: add test case for mixed quotes in xpath --- tests/keywords/test_element_security.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/keywords/test_element_security.py b/tests/keywords/test_element_security.py index c0d9f34..669b897 100644 --- a/tests/keywords/test_element_security.py +++ b/tests/keywords/test_element_security.py @@ -62,3 +62,14 @@ def test_ios_contains_match_secure(self): escaped_text = utils.escape_xpath_value(malicious_text) expected_xpath = u'//*[contains(@label,{text}) or contains(@value, {text})]'.format(text=escaped_text) self.element_keywords._element_find.assert_any_call(expected_xpath, True, True) + + def test_mixed_quotes_secure(self): + self.element_keywords._get_platform = mock.Mock(return_value='android') + self.element_keywords._element_find = mock.Mock(return_value=mock.Mock()) + malicious_text = 'a\'b"c' + + self.element_keywords._element_find_by_text(malicious_text, exact_match=True) + + escaped_text = utils.escape_xpath_value(malicious_text) + expected_xpath = u'//*[@text={}]'.format(escaped_text) + self.element_keywords._element_find.assert_any_call(expected_xpath, True, True)