| Message ID | 20211115155250.1130365-1-mkp@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] checkpatch: Detect "trojan source" attack | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
Mike Pattrick <mkp@redhat.com> writes: > Recently there has been a lot of press about the "trojan source" attack, > where Unicode characters are used to obfuscate the true functionality of > code. This attack didn't effect OVS, but adding the check here will help > guard against it sneaking in later. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- > Changes in v2: > - Now all unicode characters will result in an error. > --- I was going to suggest a checkpatch test for this - but that might result in the patch triggering itself with an error (because the robot uses the submitted version of checkpatch.py when testing). WDYT, Ilya, Gaëtan? In either case: Acked-by: Aaron Conole <aconole@redhat.com>
On 11/17/21 17:58, Aaron Conole wrote: > Mike Pattrick <mkp@redhat.com> writes: > >> Recently there has been a lot of press about the "trojan source" attack, >> where Unicode characters are used to obfuscate the true functionality of >> code. This attack didn't effect OVS, but adding the check here will help >> guard against it sneaking in later. >> >> Signed-off-by: Mike Pattrick <mkp@redhat.com> >> --- >> Changes in v2: >> - Now all unicode characters will result in an error. >> --- > > I was going to suggest a checkpatch test for this - but that might > result in the patch triggering itself with an error (because the robot > uses the submitted version of checkpatch.py when testing). > > WDYT, Ilya, Gaëtan? I think, it's good to have a test for a secutiry related functionality. And I don't think that checkpatch checks python or test files. Does it? > > In either case: > > Acked-by: Aaron Conole <aconole@redhat.com> >
Ilya Maximets <i.maximets@ovn.org> writes: > On 11/17/21 17:58, Aaron Conole wrote: >> Mike Pattrick <mkp@redhat.com> writes: >> >>> Recently there has been a lot of press about the "trojan source" attack, >>> where Unicode characters are used to obfuscate the true functionality of >>> code. This attack didn't effect OVS, but adding the check here will help >>> guard against it sneaking in later. >>> >>> Signed-off-by: Mike Pattrick <mkp@redhat.com> >>> --- >>> Changes in v2: >>> - Now all unicode characters will result in an error. >>> --- >> >> I was going to suggest a checkpatch test for this - but that might >> result in the patch triggering itself with an error (because the robot >> uses the submitted version of checkpatch.py when testing). >> >> WDYT, Ilya, Gaëtan? > > I think, it's good to have a test for a secutiry related functionality. > And I don't think that checkpatch checks python or test files. Does it? It will in some cases, but this isn't one. Okay I missed it. Yes, please add a test to tests/checkpatch.at so that we can ensure this behavior. >> >> In either case: >> >> Acked-by: Aaron Conole <aconole@redhat.com> >>
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index bf95358d5..03d91f765 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -181,6 +181,7 @@ __regex_added_doc_rst = re.compile( __regex_empty_return = re.compile(r'\s*return;') __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % __parenthesized_constructs) +__regex_nonascii_characters = re.compile("[^ -~\t]") skip_leading_whitespace_check = False skip_trailing_whitespace_check = False @@ -294,6 +295,11 @@ def pointer_whitespace_check(line): return __regex_ptr_declaration_missing_whitespace.search(line) is not None +def nonascii_character_check(line): + """Return TRUE if inappropriate Unicode characters are detected """ + return __regex_nonascii_characters.search(line) is not None + + def cast_whitespace_check(line): """Return TRUE if there is no space between the '()' used in a cast and the expression whose type is cast, i.e.: '(void *)foo'""" @@ -565,6 +571,11 @@ checks = [ 'print': lambda: print_error("Inappropriate spacing in pointer declaration")}, + {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, + 'check': lambda x: nonascii_character_check(x), + 'print': + lambda: print_error("Inappropriate non-ascii characters detected.")}, + {'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None, 'prereq': lambda x: not is_comment_line(x), 'check': lambda x: cast_whitespace_check(x),
Recently there has been a lot of press about the "trojan source" attack, where Unicode characters are used to obfuscate the true functionality of code. This attack didn't effect OVS, but adding the check here will help guard against it sneaking in later. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- Changes in v2: - Now all unicode characters will result in an error. --- utilities/checkpatch.py | 11 +++++++++++ 1 file changed, 11 insertions(+)