[vim/vim] shiftwidth value may overflow (PR #13555)

Christian Brabandt

unread,

Nov 22, 2023, 4:33:26 AM11/22/23

to vim/vim, Subscribed

The shiftwidth option values are defines as being long. However, when calculating the actual amount of indent, we cast down to (int), which may cause the shiftwidth value to become negative and later it may even cause Vim to try to allocate a huge amount of memory.

We already use long and long long variable types to calculate the indent (and detect possible overflows), so the cast to (int) seems superfluous and can be safely removed. So let's just remove the (int) cast and calculate the indent using longs.

fixes: #13554

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/13555

Commit Summary

c540a0e shiftwidth value may overflow

File Changes

(2 files)

M src/ops.c (4)
M src/testdir/test_indent.vim (9)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 11:16:34 AM11/22/23

to vim/vim, Push

@chrisbra pushed 1 commit.

9dc5b72 fix overflow on windows

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 11:33:38 AM11/22/23

to vim/vim, Push

@chrisbra pushed 1 commit.

acbf40c fix overflow in parse_cino_options

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 11:35:31 AM11/22/23

to vim/vim, Push

@chrisbra pushed 1 commit.

9e6fcaa fix overflow in parse_cino_options

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.

errael

unread,

Nov 22, 2023, 12:32:19 PM11/22/23

to vim/vim, Subscribed

@errael commented on this pull request.

In src/ops.c:

> -    int		i, j;
-    int		sw_val = (int)get_sw_value_indent(curbuf);
+    long	i, j;
+    long	sw_val = get_sw_value_indent(curbuf);

Changing sw_val, and the rest to long invalidates at least some of the overflow checking/calculation is this function (I think, probably depending on platform) for the case where sizeof(long) == sizeof(long long)

I wonder if something like the following would be useful (or exists?)

#define INT_TRIM(x) ((x) > INT_MAX ? INT_MAX : (x) < INT_MIN ? INT_MIN : (x))

Then could do

long sw_tmp = get_sw_value_indent(curbuf);
int  sw_val = INT_TRIM(sw_tmp);

In the code this PR adds to cindent.c could do

n = INT_TRIM(n);

right before the switch.

Using an inline function, rather than a macro, would be nice to avoid needing to manually create the tmp before using the macro.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 3:14:50 PM11/22/23

to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/ops.c:

> -    int		i, j;
-    int		sw_val = (int)get_sw_value_indent(curbuf);
+    long	i, j;
+    long	sw_val = get_sw_value_indent(curbuf);

Let me merge it, seems to be okay now. Please go ahead and create a PR for this followup change then.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 3:21:35 PM11/22/23

to vim/vim, Push

@chrisbra pushed 1 commit.

80344a4 fix overflow in parse_cino_options

—
View it on GitHub.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 4:23:47 PM11/22/23

to vim/vim, Subscribed

Closed #13555 via 3770574.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

errael

unread,

Nov 22, 2023, 4:29:46 PM11/22/23

to vim/vim, Subscribed

@errael commented on this pull request.

In src/ops.c:

> -    int		i, j;
-    int		sw_val = (int)get_sw_value_indent(curbuf);
+    long	i, j;
+    long	sw_val = get_sw_value_indent(curbuf);

Let me merge it, seems to be okay now. Please go ahead and create a PR for this followup change then.

OK.

(I guess that "seems to be okay" means that options are now < INT_MAX)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Nov 22, 2023, 4:32:22 PM11/22/23

to vim/vim, Subscribed

@chrisbra commented on this pull request.

In src/ops.c:

> -    int		i, j;
-    int		sw_val = (int)get_sw_value_indent(curbuf);
+    long	i, j;
+    long	sw_val = get_sw_value_indent(curbuf);

Yes, practically. I have not documented this explicitly, as I am not sure if we may need to support longs for other options specifically.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward