From 4eb65959395f02bcbc6af22f14acd58714a91e5e Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Thu, 30 May 2024 01:19:59 +0200 Subject: [PATCH 01/10] [memory] Fix read_cstring trying to read too far fix for https://github.com/hugsy/gef/issues/1055 --- gef.py | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/gef.py b/gef.py index af6ee583e..b250d94e5 100644 --- a/gef.py +++ b/gef.py @@ -10734,10 +10734,34 @@ def read_cstring(self, encoding = encoding or "unicode-escape" length = min(address | (DEFAULT_PAGE_SIZE-1), max_length+1) + while not self.get_section(address + length - 1) and length > 0: + # We want the new end address to be just before the end of the + # previous page + # - If the current end address is aligned, we want to decrease it of + # one entire page length + # - If the current end address is not aligned, we want to decrease + # it of just the offset between the page start and the current end + # address + # For doing this, we decrease the end address, and apply a page + # mask. + # + # For instance: + # - 0x555555557320 -> 0x55555555731f -> 0x555555557000 + # - 0x555555557000 -> 0x555555556fff -> 0x555555556000 + + new_end_addr = address + length - 1 + page_mask = ((1 << gef.arch.ptrsize * 8) - 1) ^ (DEFAULT_PAGE_SIZE - 1) + + length = (new_end_addr & page_mask) - address + + if length <= 0: + err(f"Can't read memory at '{address:#x}'") + return "" + try: res_bytes = self.read(address, length) except gdb.error: - err(f"Can't read memory at '{address}'") + err(f"Can't read memory at '{address:#x}'") return "" try: with warnings.catch_warnings(): @@ -10767,6 +10791,13 @@ def maps(self) -> List[Section]: self.__maps = self.__parse_maps() return self.__maps + def get_section(self, addr: int) -> Optional[Section]: + """Return the section of the provided address if it exists, `None` + otherwise.""" + for section in self.maps: + if section.page_start <= addr and section.page_end > addr: + return section + def __parse_maps(self) -> Optional[List[Section]]: """Return the mapped memory sections. If the current arch has its maps method defined, then defer to that to generated maps, otherwise, try to From effaa16b9551053e81a96882e1b3c532891d6e2d Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Thu, 30 May 2024 02:58:39 +0200 Subject: [PATCH 02/10] Page mask can be simplified --- gef.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gef.py b/gef.py index b250d94e5..5235d06f8 100644 --- a/gef.py +++ b/gef.py @@ -10750,7 +10750,7 @@ def read_cstring(self, # - 0x555555557000 -> 0x555555556fff -> 0x555555556000 new_end_addr = address + length - 1 - page_mask = ((1 << gef.arch.ptrsize * 8) - 1) ^ (DEFAULT_PAGE_SIZE - 1) + page_mask = ~(DEFAULT_PAGE_SIZE - 1) length = (new_end_addr & page_mask) - address From 89bd5c7adfe1a0569e358f340f8aeadceabbb2c1 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Thu, 19 Sep 2024 10:20:14 +0200 Subject: [PATCH 03/10] Revert "Page mask can be simplified" This reverts commit effaa16b9551053e81a96882e1b3c532891d6e2d. --- gef.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gef.py b/gef.py index 5235d06f8..b250d94e5 100644 --- a/gef.py +++ b/gef.py @@ -10750,7 +10750,7 @@ def read_cstring(self, # - 0x555555557000 -> 0x555555556fff -> 0x555555556000 new_end_addr = address + length - 1 - page_mask = ~(DEFAULT_PAGE_SIZE - 1) + page_mask = ((1 << gef.arch.ptrsize * 8) - 1) ^ (DEFAULT_PAGE_SIZE - 1) length = (new_end_addr & page_mask) - address From 21e2699a020eb966f5521d4725693bfd6dd215a4 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Thu, 19 Sep 2024 10:20:27 +0200 Subject: [PATCH 04/10] Revert "[memory] Fix read_cstring trying to read too far" This reverts commit 4eb65959395f02bcbc6af22f14acd58714a91e5e. --- gef.py | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/gef.py b/gef.py index b250d94e5..af6ee583e 100644 --- a/gef.py +++ b/gef.py @@ -10734,34 +10734,10 @@ def read_cstring(self, encoding = encoding or "unicode-escape" length = min(address | (DEFAULT_PAGE_SIZE-1), max_length+1) - while not self.get_section(address + length - 1) and length > 0: - # We want the new end address to be just before the end of the - # previous page - # - If the current end address is aligned, we want to decrease it of - # one entire page length - # - If the current end address is not aligned, we want to decrease - # it of just the offset between the page start and the current end - # address - # For doing this, we decrease the end address, and apply a page - # mask. - # - # For instance: - # - 0x555555557320 -> 0x55555555731f -> 0x555555557000 - # - 0x555555557000 -> 0x555555556fff -> 0x555555556000 - - new_end_addr = address + length - 1 - page_mask = ((1 << gef.arch.ptrsize * 8) - 1) ^ (DEFAULT_PAGE_SIZE - 1) - - length = (new_end_addr & page_mask) - address - - if length <= 0: - err(f"Can't read memory at '{address:#x}'") - return "" - try: res_bytes = self.read(address, length) except gdb.error: - err(f"Can't read memory at '{address:#x}'") + err(f"Can't read memory at '{address}'") return "" try: with warnings.catch_warnings(): @@ -10791,13 +10767,6 @@ def maps(self) -> List[Section]: self.__maps = self.__parse_maps() return self.__maps - def get_section(self, addr: int) -> Optional[Section]: - """Return the section of the provided address if it exists, `None` - otherwise.""" - for section in self.maps: - if section.page_start <= addr and section.page_end > addr: - return section - def __parse_maps(self) -> Optional[List[Section]]: """Return the mapped memory sections. If the current arch has its maps method defined, then defer to that to generated maps, otherwise, try to From c722f5939e1df07a0ccaf0fc3d760f846ef2851a Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Thu, 19 Sep 2024 10:46:37 +0200 Subject: [PATCH 05/10] [memory] Fix read_cstring trying to read too far --- gef.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/gef.py b/gef.py index af6ee583e..6f9d55601 100644 --- a/gef.py +++ b/gef.py @@ -10737,8 +10737,22 @@ def read_cstring(self, try: res_bytes = self.read(address, length) except gdb.error: - err(f"Can't read memory at '{address}'") - return "" + current_address = address + res_bytes = b"" + while len(res_bytes) < length: + try: + new_end_addr = current_address + DEFAULT_PAGE_SIZE + page_mask = ~(DEFAULT_PAGE_SIZE - 1) + size = (new_end_addr & page_mask) - current_address + + res_bytes += self.read(current_address, size) + + current_address += size + except: + if not res_bytes: + err(f"Can't read memory at '{address}'") + return "" + break try: with warnings.catch_warnings(): # ignore DeprecationWarnings (see #735) From a9034646d827ee305585e5ee669ed0664d95a597 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Mon, 23 Sep 2024 18:09:26 +0200 Subject: [PATCH 06/10] Rename variable Fix https://github.com/hugsy/gef/pull/1112#discussion_r1770559412 --- gef.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gef.py b/gef.py index 6f9d55601..e9e9d8281 100644 --- a/gef.py +++ b/gef.py @@ -10741,9 +10741,9 @@ def read_cstring(self, res_bytes = b"" while len(res_bytes) < length: try: - new_end_addr = current_address + DEFAULT_PAGE_SIZE + next_page = current_address + DEFAULT_PAGE_SIZE page_mask = ~(DEFAULT_PAGE_SIZE - 1) - size = (new_end_addr & page_mask) - current_address + size = (next_page & page_mask) - current_address res_bytes += self.read(current_address, size) From 9664987ad39b06a317f8bbee38bdd9b894a1c873 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Mon, 23 Sep 2024 18:10:53 +0200 Subject: [PATCH 07/10] Add comment about mask calculation Fix https://github.com/hugsy/gef/pull/1112#discussion_r1770559499 --- gef.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gef.py b/gef.py index e9e9d8281..002736c93 100644 --- a/gef.py +++ b/gef.py @@ -10741,10 +10741,12 @@ def read_cstring(self, res_bytes = b"" while len(res_bytes) < length: try: + # Calculate how much bytes there is until next page next_page = current_address + DEFAULT_PAGE_SIZE page_mask = ~(DEFAULT_PAGE_SIZE - 1) size = (next_page & page_mask) - current_address + # Read until the end of the current page res_bytes += self.read(current_address, size) current_address += size From 376a07ac3f9f8563f9750231001bd211dd94b476 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Mon, 23 Sep 2024 18:34:41 +0200 Subject: [PATCH 08/10] Add test --- tests/api/gef_memory.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/api/gef_memory.py b/tests/api/gef_memory.py index 8a448a5af..6b0b6760d 100644 --- a/tests/api/gef_memory.py +++ b/tests/api/gef_memory.py @@ -178,3 +178,17 @@ def test_func_parse_maps_realpath(self): "/usr" not in section.realpath): assert pathlib.Path(section.realpath).is_file() break + + def test_func_read_cstring_oob(self): + gef, gdb = self._gef, self._gdb + + gdb.execute("b main") + gdb.execute("start") + + section = gef.memory.maps[0] + oob_val = gef.memory.read_cstring(section.page_start, section.page_end - + section.page_start + 0x100) + val = gef.memory.read_cstring(section.page_start, section.page_end - + section.page_start) + + assert val == oob_val From 2ef7ed84d5efac9f4719dc74b694ac67a5954284 Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Tue, 24 Sep 2024 09:24:26 +0200 Subject: [PATCH 09/10] nit Co-authored-by: Grazfather --- gef.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gef.py b/gef.py index 002736c93..9b0dd4b73 100644 --- a/gef.py +++ b/gef.py @@ -10741,7 +10741,7 @@ def read_cstring(self, res_bytes = b"" while len(res_bytes) < length: try: - # Calculate how much bytes there is until next page + # Calculate how many bytes there are until next page next_page = current_address + DEFAULT_PAGE_SIZE page_mask = ~(DEFAULT_PAGE_SIZE - 1) size = (next_page & page_mask) - current_address From 7d092564ead06aa28678855dddc286988383069c Mon Sep 17 00:00:00 2001 From: ValekoZ Date: Fri, 27 Sep 2024 14:26:35 +0200 Subject: [PATCH 10/10] fix nits --- gef.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gef.py b/gef.py index 9b0dd4b73..2a6312ae8 100644 --- a/gef.py +++ b/gef.py @@ -10750,9 +10750,9 @@ def read_cstring(self, res_bytes += self.read(current_address, size) current_address += size - except: + except gdb.error: if not res_bytes: - err(f"Can't read memory at '{address}'") + err(f"Can't read memory at '{address:#x}'") return "" break try: