Skip to content

Avoid out-of bounds read access issue introduced with#1111

Closed
chcg wants to merge 1 commit intonotepad-plus-plus:masterfrom
chcg:SmartHighLight-out-of-bounds-access
Closed

Avoid out-of bounds read access issue introduced with#1111
chcg wants to merge 1 commit intonotepad-plus-plus:masterfrom
chcg:SmartHighLight-out-of-bounds-access

Conversation

@chcg
Copy link
Copy Markdown
Contributor

@chcg chcg commented Nov 9, 2015

Bugfix on PR Fix smarthighlight #187
, see also comments added in the PR.

String returned by SCI_GETWORDCHARS from scintilla is not null terminated, so check for strlen in isWordChar() below on listChar is dangerous as strlen accesses data after the buffer until the first following null is found in memory

  • seen with MS Application Verifier on x64 release
  • expected to also happen on win32 x86 release
  • for debug typically the memory is preallocated with zero, so this issue is maybe not visible for debug builds

https://github.com/notepad-plus-plus/notepad-plus-plus/pull/187/files
, see also comments added in the PR:

String returned by SCI_GETWORDCHARS from scintilla is not null terminated, so check for strlen in isWordChar() below on listChar is dangerous as strlen accesses data after the buffer until the first following null is found in memory
- seen with MS Application Verifier on x64 release
- expected to also happen on win32 x86 release
@Cyrillev91
Copy link
Copy Markdown
Contributor

I agree with you.

@ariccio
Copy link
Copy Markdown
Contributor

ariccio commented Nov 14, 2015

(!!!) I hit this too!

(I'll create the issue)

@ariccio
Copy link
Copy Markdown
Contributor

ariccio commented Nov 14, 2015

See also, my four comments on the commit: 037b41e#commitcomment-14399645

ariccio added a commit to ariccio/notepad-plus-plus that referenced this pull request Nov 15, 2015
The original code was unnecessarily complex, did unnecessary work, and
overran the listChar buffer. This is an alternative to notepad-plus-plus#1111, with more
const, and simpler code.
@donho donho added the accepted label Nov 18, 2015
@donho donho added this to the 6.x milestone Nov 18, 2015
@donho donho closed this in 71edfb2 Nov 18, 2015
@chcg chcg deleted the SmartHighLight-out-of-bounds-access branch January 2, 2021 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants