In vi, the command 'B' allows to jump to the beginning of the previous
word. Due to insufficient checks, its handling can causes currently two
out-of-bounds memory accesses.
Generally, vi allocates a buffer for the text held by the current editor
session. The char pointer 'text' points to the start of the buffer, the
char pointer 'dot' is a pointer to the current cursor location within
the allocated buffer.
When handling the 'B' command (together with 'E' and 'W') in 'do_cmd',
the condition 'isspace(dot[dir])' is checked, where 'dir == -1' for
'B'. If 'dot' points to the start of the text buffer, so 'dot == text',
this access is out of bounds. Add a check to make sure that the
relative access of 'dot' is not before the start of the buffer.
During further handling of the 'B' command, a similar out of bounds
access happens when calculating the new 'dot'. The function 'skip_thing'
invokes 'st_test' with a current pointer 'p' and a direction 'dir'. In
the same way, 'p[inc]' is accessed without previously checking whether
this memory is inside the allocated text buffer. Move the bounds check
before the invocation of 'st_test' into the while condition, together
with other checks. This also shrinks the code a bit.
On CHERI systems with fine-grained memory protection through
architectural capabilities, these out of bounds accesses caused bounds
violations.
function old new delta
do_cmd 4798 4806 +8
skip_thing 281 262 -19
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/1 up/down: 8/-19) Total: -11 bytes
Signed-off-by: Chris Hofer <christian.ho...@codasip.com>
---
editors/vi.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/editors/vi.c b/editors/vi.c
index ca8541eb5..f2d3e0ac5 100644
--- a/editors/vi.c
+++ b/editors/vi.c
@@ -3502,16 +3502,15 @@ static char *skip_thing(char *p, int linecnt, int dir,
int type)
{
char c;
- while (st_test(p, type, dir, &c)) {
+ while
+ (
+ (dir >= 0 ? p < end - 1 : p > text) &&
+ st_test(p, type, dir, &c) &&
// make sure we limit search to correct number of lines
- if (c == '\n' && --linecnt < 1)
- break;
- if (dir >= 0 && p >= end - 1)
- break;
- if (dir < 0 && p <= text)
- break;
+ (c != '\n' || --linecnt >= 1)
+ )
p += dir; // move to next char
- }
+
return p;
}
@@ -4180,7 +4179,7 @@ static void do_cmd(int c)
if (c == 'B')
dir = BACK;
do {
- if (c == 'W' || isspace(dot[dir])) {
+ if (c == 'W' || (&dot[dir] >= text &&
isspace(dot[dir]))) {
dot = skip_thing(dot, 1, dir, S_TO_WS);
dot = skip_thing(dot, 2, dir, S_OVER_WS);
}
--
2.47.1
_______________________________________________
busybox mailing list
busybox@busybox.net
https://lists.busybox.net/mailman/listinfo/busybox
