Author Topic: Small patch for out of bounds warning.  (Read 8076 times)

Offline Smitty

  • Multiple posting newcomer
  • *
  • Posts: 11
Small patch for out of bounds warning.
« on: May 20, 2021, 09:32:45 am »
Here is a little patch to correct a warning - a legitimate warning.
I looked at submitting a patch in the prescribed fashion, but frankly, the instructions were so complicated, that I decided I couldn't be bothered.
I think that should be addressed - it should be mutch easier for anyone to submit patches, or merge requests.

Code
src/scintilla/lexers/LexMMIXAL.cxx

Index: wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx
===================================================================
--- wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx (revision 12450)
+++ wxscintilla/src/scintilla/lexers/LexMMIXAL.cxx (working copy)
@@ -103,7 +103,7 @@
  char s[100];
  sc.GetCurrent(s, sizeof(s));
  if (*s == ':') { // ignore base prefix for match
-                                       for (size_t i = 0; i != sizeof(s); ++i) {
+                                       for (size_t i = 0; i != sizeof(s[0]); ++i) {
  *(s+i) = *(s+i+1);
                                        }
  }

Offline sodev

  • Regular
  • ***
  • Posts: 497
Re: Small patch for out of bounds warning.
« Reply #1 on: May 20, 2021, 09:57:36 am »
Not sure what the warning was, most probably about accessing an index out of bounds, but your patch does not fix the code, it actually breaks the code. The code fragment removes a leading colon from a string by shifting down the content by one, sizeof(s) == 100, sizeof(s[0]) == 1, after your patch the colon gets overwritten with the following character only. A proper fix would be to use sizeof(s) - 1 in the condition.

Offline Miguel Gimenez

  • Developer
  • Lives here!
  • *****
  • Posts: 1553
Re: Small patch for out of bounds warning.
« Reply #2 on: May 20, 2021, 10:18:14 am »
Or use
Code
memmove(s, s+1, sizeof(s)-1)

Offline omlk

  • Multiple posting newcomer
  • *
  • Posts: 110
Re: Small patch for out of bounds warning.
« Reply #3 on: May 20, 2021, 11:44:15 am »
Or use
Code
memmove(s, s+1, sizeof(s)-1)

Code
 
s[100] = "123456789";
s[0] = '1'; ...; s[8] = '9'; s[last] = '\0';

char s[100]; //WCHAR s[100];
char s2[1000];
unsigned char_array_size_bytes = sizeof(s);
unsigned char char_size_bytes = sizeof(s[0]);
unsigned char_counts = char_array_size_bytes /char_size_bytes;
memmove(s2+5, s, strlen(s)+1)
« Last Edit: May 20, 2021, 12:31:16 pm by omlk »

Offline Smitty

  • Multiple posting newcomer
  • *
  • Posts: 11
Re: Small patch for out of bounds warning.
« Reply #4 on: May 20, 2021, 12:04:46 pm »
I had some problems with pasting my patch into this forum, and somehow I corrupted it - I apologize, I didn't notice.

Absolutely correct, that I have broken almost correct code.

Aside from the fact that there are better ways of doing this, it will cause a buffer over-run warning (on most standard compilers)

Granted, at compile time the sizeof(s) is computed correctly - since s is stack based - this is not an error.  But, warnings should not be allowed to persist.

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Small patch for out of bounds warning.
« Reply #5 on: May 20, 2021, 08:07:22 pm »
I looked at submitting a patch in the prescribed fashion, but frankly, the instructions were so complicated, that I decided I couldn't be bothered.
Which instructions?
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline stahta01

  • Lives here!
  • ****
  • Posts: 7582
    • My Best Post
Re: Small patch for out of bounds warning.
« Reply #6 on: May 20, 2021, 08:58:06 pm »
These directions are long; but, I see no reason to believe they are hard or too complex to follow.

https://wiki.codeblocks.org/index.php/Creating_a_patch_to_submit_(Patch_Tracker)

But, who knows which directions the original poster (OP) used.

Tim S.
C Programmer working to learn more about C++ and Git.
On Windows 7 64 bit and Windows 10 64 bit.
--
When in doubt, read the CB WiKi FAQ. http://wiki.codeblocks.org

Offline grem

  • Multiple posting newcomer
  • *
  • Posts: 22
Re: Small patch for out of bounds warning.
« Reply #7 on: March 09, 2024, 08:11:44 pm »

Offline grem

  • Multiple posting newcomer
  • *
  • Posts: 22
Re: Small patch for out of bounds warning.
« Reply #8 on: March 09, 2024, 08:50:38 pm »
Attached file "Scintilla_fix_buffer_over-read_with_absolute_reference.patch" contains prepared patch that is taken from upstream fix.

I created patch-ticket: https://sourceforge.net/p/codeblocks/tickets/1463/
« Last Edit: March 10, 2024, 09:28:18 am by grem »

Offline grem

  • Multiple posting newcomer
  • *
  • Posts: 22
Re: Small patch for out of bounds warning.
« Reply #9 on: March 14, 2024, 11:31:44 pm »
Applied in https://sourceforge.net/p/codeblocks/code/13491/

Thanks to Miguel Gimenez.