RE: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RE: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US)
CLASSIFICATION: UNCLASSIFIED

I tried to create test cases in several compilers (old GNU, icc, clang), and every one resulted in binaries that only evaluated the "switch" statement once before building the jump hierarchy (against that singular calculation) for the catch blocks. The only time this was not the case was when I defined custom SWITCH and CASE macros.

We have had customers define custom case and switch macros when, for example, they have dynamic or string values to build the CASE structure. Consider the header file switchs.h available at https://stackoverflow.com/a/29605146/4930736

We've encountered "switch" statements like the attached that demonstrates this CWE can affect standard C and C++ code.

**PLEASE NOTE: While the driver is custom code, the switchs.h file is shamelessly ripped from https://gist.githubusercontent.com/HoX/abfe15c40f2d9daebc35/raw/aa88c9b304dc9338041abcd5cddb40fc689fe5aa/switchs.h and I don't want anyone to imply that this github/stackoverflow user is anything less than a stellar developer who found an excellent solution for a common problem in C/C++. Just because someone can abuse this solution doesn't mean that it's not a good one.

Jon

Dr. Jonathan Hood
Chief Engineer | AMRDEC/S3I — Lifecycle Cyber Engineering Branch
☎ (256) 876-0326

-----Original Message-----
From: Fulvio Baccaglini [mailto:[hidden email]]
Sent: Wednesday, September 12, 2018 5:33 AM
To: CWE Research Discussion <[hidden email]>
Subject: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++

Hi Drew,

Tracking those C11 paragraphs back to earlier C standards:

ISO C99 has the same wording and paragraph numbers as ISO C11:
[C99-6.8.4.2-5] and [C99-6.8.6.3-2]. See for instance draft N1256 from the ISO C WG14 standards page.

ISO C90 also has the same wording, except that it uses the term "integral promotions" instead of "integer promotions" to express the same concept. Section references are:
[C90-6.6.4.2] and [C90-6.6.6.3].

Before ISO C there was the ANSI C standard (1989), and my understanding is that it can be considered equivalent to ISO C90.

Fulvio


On Tue, 2018-09-11 at 16:54 +0000, Buttner, Drew wrote:

> Is there anyone else on the list that can confirm or shed some light
> onto this issue?  The CWE was taken from CLASP back in the early days
> of CWE.  Is it possible that this weakness affected earlier versions
> of C?
>
> Thanks
> Drew
>
>
> -----Original Message-----
> From: Fulvio Baccaglini <[hidden email]>
> Sent: Friday, August 31, 2018 9:54 AM
> To: CWE Research Discussion <[hidden email]>
> Subject: CWE-365 "Race Condition in Switch" is not applicable to C/C++
>
> The description of CWE-365 is "The code contains a switch statement in
> which the switched variable can be modified while the switch is still
> executing, resulting in unexpected behavior."
>
> This description, and the Demonstrative Example 1 for C, suggest that
> the controlling expression of a switch statement can be evaluated
> multiple times.
>
> However, the applicable paragraphs from the ISO C11 standard suggest
> that it is evaluated only once:
>
> [C11-6.8.4.2-5] The switch statement - "The integer promotions are
> performed on the controlling expression. The constant expression in
> each case label is converted to the promoted type of the controlling
> expression. If a converted value matches that of the promoted
> controlling expression, control jumps to the statement following the
> matched case label. Otherwise, if there is a default label, control
> jumps to the labeled statement. If no converted case constant
> expression matches and there is no default label, no part of the
> switch body is executed."
>
> [C11-6.8.6.3] The break statement - "A break statement terminates
> execution of the smallest enclosing switch or iteration statement."
>
> This is also the case for C++ (so that CWE-365 should not be part of
> the
> CWE-659 View: Weaknesses in Software Written in C++).
>
> (I am advised that also in Java and C# the controlling expression
> would be evaluated only once).
>
> Fulvio
>
>
CLASSIFICATION: UNCLASSIFIED

switchs_driver.cpp (646 bytes) Download Attachment
switchs.h (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US)
CLASSIFICATION: UNCLASSIFIED

Sorry; I originally wrote a reply (included below this response) and was informed that our mail servers dropped the message because of the attachments.

Attachments were switchs.h from https://gist.githubusercontent.com/HoX/abfe15c40f2d9daebc35/raw/aa88c9b304dc9338041abcd5cddb40fc689fe5aa/switchs.h

And switchs_driver.cpp:
#include <iostream>
#include "switchs.h"

using namespace std;

char * ChangeTheMucus(char *test)
{
        static char ret[] = "boogie";
        strcpy(test, "snotty");
        return ret;
}

int main()
{
        char test[] = "booger";
        switchs(test)
        {
                cases(ChangeTheMucus(test))
                        cout << "Skip Me!" << endl;
                        break;
                cases("snotty")
                        cout << "Uh oh, we got here..." << endl;
                        break;
                defaults
                        cout << "We should have gotten here." << endl;
                        break;
        } switchs_end;
        return 0;
}

Jon

Dr. Jonathan Hood
Chief Engineer | AMRDEC/S3I — Lifecycle Cyber Engineering Branch
☎ (256) 876-0326


-----Original Message-----
From: Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US)
Sent: Wednesday, September 12, 2018 8:43 AM
To: 'Fulvio Baccaglini' <[hidden email]>; CWE Research Discussion <[hidden email]>
Subject: RE: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

CLASSIFICATION: UNCLASSIFIED

I tried to create test cases in several compilers (old GNU, icc, clang), and every one resulted in binaries that only evaluated the "switch" statement once before building the jump hierarchy (against that singular calculation) for the catch blocks. The only time this was not the case was when I defined custom SWITCH and CASE macros.

We have had customers define custom case and switch macros when, for example, they have dynamic or string values to build the CASE structure. Consider the header file switchs.h available at https://stackoverflow.com/a/29605146/4930736

We've encountered "switch" statements like the attached that demonstrates this CWE can affect standard C and C++ code.

**PLEASE NOTE: While the driver is custom code, the switchs.h file is shamelessly ripped from https://gist.githubusercontent.com/HoX/abfe15c40f2d9daebc35/raw/aa88c9b304dc9338041abcd5cddb40fc689fe5aa/switchs.h and I don't want anyone to imply that this github/stackoverflow user is anything less than a stellar developer who found an excellent solution for a common problem in C/C++. Just because someone can abuse this solution doesn't mean that it's not a good one.

Jon

Dr. Jonathan Hood
Chief Engineer | AMRDEC/S3I — Lifecycle Cyber Engineering Branch ☎ (256) 876-0326

-----Original Message-----
From: Fulvio Baccaglini [mailto:[hidden email]]
Sent: Wednesday, September 12, 2018 5:33 AM
To: CWE Research Discussion <[hidden email]>
Subject: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++

Hi Drew,

Tracking those C11 paragraphs back to earlier C standards:

ISO C99 has the same wording and paragraph numbers as ISO C11:
[C99-6.8.4.2-5] and [C99-6.8.6.3-2]. See for instance draft N1256 from the ISO C WG14 standards page.

ISO C90 also has the same wording, except that it uses the term "integral promotions" instead of "integer promotions" to express the same concept. Section references are:
[C90-6.6.4.2] and [C90-6.6.6.3].

Before ISO C there was the ANSI C standard (1989), and my understanding is that it can be considered equivalent to ISO C90.

Fulvio


On Tue, 2018-09-11 at 16:54 +0000, Buttner, Drew wrote:

> Is there anyone else on the list that can confirm or shed some light
> onto this issue?  The CWE was taken from CLASP back in the early days
> of CWE.  Is it possible that this weakness affected earlier versions
> of C?
>
> Thanks
> Drew
>
>
> -----Original Message-----
> From: Fulvio Baccaglini <[hidden email]>
> Sent: Friday, August 31, 2018 9:54 AM
> To: CWE Research Discussion <[hidden email]>
> Subject: CWE-365 "Race Condition in Switch" is not applicable to C/C++
>
> The description of CWE-365 is "The code contains a switch statement in
> which the switched variable can be modified while the switch is still
> executing, resulting in unexpected behavior."
>
> This description, and the Demonstrative Example 1 for C, suggest that
> the controlling expression of a switch statement can be evaluated
> multiple times.
>
> However, the applicable paragraphs from the ISO C11 standard suggest
> that it is evaluated only once:
>
> [C11-6.8.4.2-5] The switch statement - "The integer promotions are
> performed on the controlling expression. The constant expression in
> each case label is converted to the promoted type of the controlling
> expression. If a converted value matches that of the promoted
> controlling expression, control jumps to the statement following the
> matched case label. Otherwise, if there is a default label, control
> jumps to the labeled statement. If no converted case constant
> expression matches and there is no default label, no part of the
> switch body is executed."
>
> [C11-6.8.6.3] The break statement - "A break statement terminates
> execution of the smallest enclosing switch or iteration statement."
>
> This is also the case for C++ (so that CWE-365 should not be part of
> the
> CWE-659 View: Weaknesses in Software Written in C++).
>
> (I am advised that also in Java and C# the controlling expression
> would be evaluated only once).
>
> Fulvio
>
>

CLASSIFICATION: UNCLASSIFIED
CLASSIFICATION: UNCLASSIFIED
Reply | Threaded
Open this post in threaded view
|

Re: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

Steve Grubb
In reply to this post by Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US)
On Wednesday, September 12, 2018 9:43:03 AM EDT Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US) wrote:
> CLASSIFICATION: UNCLASSIFIED
>
> I tried to create test cases in several compilers (old GNU, icc, clang),
> and every one resulted in binaries that only evaluated the "switch"
> statement once before building the jump hierarchy (against that singular
> calculation) for the catch blocks. The only time this was not the case was
> when I defined custom SWITCH and CASE macros.
 
> We have had customers define custom case and switch macros when, for
> example, they have dynamic or string values to build the CASE structure.
> Consider the header file switchs.h available at
> https://stackoverflow.com/a/29605146/4930736

While that is an interesting example, when preprocessed, it yields a program
full of 'if' statements which the expression in the 'if' is expected to be
evaluated each time.


$ g++ -E test.cpp

using namespace std;

char * ChangeTheMucus(char *test)
{
        static char ret[] = "boogie";
        strcpy(test, "snotty");
        return ret;
}

int main()
{
        char test[] = "booger";
        { char *__sw = (test); bool __done = false; bool __cont = false; regex_t __regex; regcomp(&__regex, ".*", 0); do {
        {
                } if ( __cont || !strcmp ( __sw, ChangeTheMucus(test) ) ) { __done = true; __cont = true;
                        cout << "Skip Me!" << endl;
                        break;
                } if ( __cont || !strcmp ( __sw, "snotty" ) ) { __done = true; __cont = true;
                        cout << "Uh oh, we got here..." << endl;
                        break;
                } if ( !__done || __cont ) {
                        cout << "We should have gotten here." << endl;
                        break;
        } } while ( 0 ); regfree(&__regex); };
        return 0;
}



K&R defines a switch/case construct as a multiway decision that matches
against integer values. I think there is an implied atomicity where the
switch expression is evaluated once and that result is then matched. It does
allow for a constant expression. But then a constant expression is something
that is evaluated at compile time such as substituting the value of a 'define'
into the code that uses it.

-Steve


> We've encountered "switch" statements like the attached that demonstrates
> this CWE can affect standard C and C++ code.
 
> **PLEASE NOTE: While the driver is custom code, the switchs.h file is
> shamelessly ripped from
> https://gist.githubusercontent.com/HoX/abfe15c40f2d9daebc35/raw/aa88c9b304
> dc9338041abcd5cddb40fc689fe5aa/switchs.h and I don't want anyone to imply
> that this github/stackoverflow user is anything less than a stellar
> developer who found an excellent solution for a common problem in C/C++.
> Just because someone can abuse this solution doesn't mean that it's not a
> good one.
 

> Jon
>
> Dr. Jonathan Hood
> Chief Engineer | AMRDEC/S3I — Lifecycle Cyber Engineering Branch
> ☎ (256) 876-0326
>
> -----Original Message-----
> From: Fulvio Baccaglini [mailto:[hidden email]]
> Sent: Wednesday, September 12, 2018 5:33 AM
> To: CWE Research Discussion <[hidden email]>
> Subject: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not
> applicable to C/C++
 
> Hi Drew,
>
> Tracking those C11 paragraphs back to earlier C standards:
>
> ISO C99 has the same wording and paragraph numbers as ISO C11:
> [C99-6.8.4.2-5] and [C99-6.8.6.3-2]. See for instance draft N1256 from the
> ISO C WG14 standards page.
 
> ISO C90 also has the same wording, except that it uses the term "integral
> promotions" instead of "integer promotions" to express the same concept.
> Section references are:
 [C90-6.6.4.2] and [C90-6.6.6.3].
>
> Before ISO C there was the ANSI C standard (1989), and my understanding is
> that it can be considered equivalent to ISO C90.
 

> Fulvio
>
>
> On Tue, 2018-09-11 at 16:54 +0000, Buttner, Drew wrote:
>
> > Is there anyone else on the list that can confirm or shed some light
> > onto this issue?  The CWE was taken from CLASP back in the early days
> > of CWE.  Is it possible that this weakness affected earlier versions
> > of C?
> >
> > Thanks
> > Drew
> >
> >
> > -----Original Message-----
> > From: Fulvio Baccaglini <[hidden email]>
> > Sent: Friday, August 31, 2018 9:54 AM
> > To: CWE Research Discussion <[hidden email]>
> > Subject: CWE-365 "Race Condition in Switch" is not applicable to C/C++
> >
> > The description of CWE-365 is "The code contains a switch statement in
> > which the switched variable can be modified while the switch is still
> > executing, resulting in unexpected behavior."
> >
> > This description, and the Demonstrative Example 1 for C, suggest that
> > the controlling expression of a switch statement can be evaluated
> > multiple times.
> >
> > However, the applicable paragraphs from the ISO C11 standard suggest
> > that it is evaluated only once:
> >
> > [C11-6.8.4.2-5] The switch statement - "The integer promotions are
> > performed on the controlling expression. The constant expression in
> > each case label is converted to the promoted type of the controlling
> > expression. If a converted value matches that of the promoted
> > controlling expression, control jumps to the statement following the
> > matched case label. Otherwise, if there is a default label, control
> > jumps to the labeled statement. If no converted case constant
> > expression matches and there is no default label, no part of the
> > switch body is executed."
> >
> > [C11-6.8.6.3] The break statement - "A break statement terminates
> > execution of the smallest enclosing switch or iteration statement."
> >
> > This is also the case for C++ (so that CWE-365 should not be part of
> > the
> > CWE-659 View: Weaknesses in Software Written in C++).
> >
> > (I am advised that also in Java and C# the controlling expression
> > would be evaluated only once).
> >
> > Fulvio
> >
> >
>
>
> CLASSIFICATION: UNCLASSIFIED





Reply | Threaded
Open this post in threaded view
|

RE: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US)
CLASSIFICATION: UNCLASSIFIED

I understand that the syntax doesn't expand to meet the C definition of a switch statement, and I appreciate the Kernighan & Ritchie definition. If the developers are using the C(++) switch/case statements, I agree that this CWE is moot by definition.

Semantically, I've run into developers dropping in these switch-like macros that are vulnerable to CWE-365, intending for them to function in the same way as switch/case statements. Perhaps CWE-367 (TOCTOU) is a more correct CWE to use for this particular issue, but I've found that pointing to CWE-365 helps developers understand the issue more and change the way they think about how switch statements should work.

I feel sorry for the CWE maintainer who has to decide among (1) removing C(++) as an applicable language, (2) leaving it in since developers associate a different meaning with these switch-like statements, or (3) expand the CWE to cover switch-like statements or custom switch structure implementations. I feel like we could make good arguments for any of these.

Jon

Dr. Jonathan Hood
Chief Engineer S3E Contract | AMRDEC/S3I — Lifecycle Cyber Engineering Branch
☎ (256) 876-0326


-----Original Message-----
From: Steve Grubb [mailto:[hidden email]]
Sent: Monday, September 17, 2018 8:13 AM
To: Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US) <[hidden email]>
Cc: Fulvio Baccaglini <[hidden email]>; CWE Research Discussion <[hidden email]>
Subject: Re: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is not applicable to C/C++ (UNCLASSIFIED)

All active links contained in this email were disabled.  Please verify the identity of the sender, and confirm the authenticity of all links contained within the message prior to copying and pasting the address to a Web browser.  




----

On Wednesday, September 12, 2018 9:43:03 AM EDT Hood, Jonathan W CTR USARMY RDECOM AMRDEC (US) wrote:
> CLASSIFICATION: UNCLASSIFIED
>
> I tried to create test cases in several compilers (old GNU, icc,
> clang), and every one resulted in binaries that only evaluated the "switch"
> statement once before building the jump hierarchy (against that
> singular
> calculation) for the catch blocks. The only time this was not the case
> was when I defined custom SWITCH and CASE macros.
 
> We have had customers define custom case and switch macros when, for
> example, they have dynamic or string values to build the CASE structure.
> Consider the header file switchs.h available at
> Caution-https://stackoverflow.com/a/29605146/4930736

While that is an interesting example, when preprocessed, it yields a program full of 'if' statements which the expression in the 'if' is expected to be evaluated each time.


$ g++ -E test.cpp

using namespace std;

char * ChangeTheMucus(char *test)
{
        static char ret[] = "boogie";
        strcpy(test, "snotty");
        return ret;
}

int main()
{
        char test[] = "booger";
        { char *__sw = (test); bool __done = false; bool __cont = false; regex_t __regex; regcomp(&__regex, ".*", 0); do {
        {
                } if ( __cont || !strcmp ( __sw, ChangeTheMucus(test) ) ) { __done = true; __cont = true;
                        cout << "Skip Me!" << endl;
                        break;
                } if ( __cont || !strcmp ( __sw, "snotty" ) ) { __done = true; __cont = true;
                        cout << "Uh oh, we got here..." << endl;
                        break;
                } if ( !__done || __cont ) {
                        cout << "We should have gotten here." << endl;
                        break;
        } } while ( 0 ); regfree(&__regex); };
        return 0;
}



K&R defines a switch/case construct as a multiway decision that matches against integer values. I think there is an implied atomicity where the switch expression is evaluated once and that result is then matched. It does allow for a constant expression. But then a constant expression is something that is evaluated at compile time such as substituting the value of a 'define'
into the code that uses it.

-Steve


> We've encountered "switch" statements like the attached that
> demonstrates this CWE can affect standard C and C++ code.
 
> **PLEASE NOTE: While the driver is custom code, the switchs.h file is
> shamelessly ripped from
> Caution-https://gist.githubusercontent.com/HoX/abfe15c40f2d9daebc35/ra
> w/aa88c9b304 dc9338041abcd5cddb40fc689fe5aa/switchs.h and I don't want
> anyone to imply that this github/stackoverflow user is anything less
> than a stellar developer who found an excellent solution for a common
> problem in C/C++.
> Just because someone can abuse this solution doesn't mean that it's
> not a good one.
 

> Jon
>
> Dr. Jonathan Hood
> Chief Engineer | AMRDEC/S3I — Lifecycle Cyber Engineering Branch ☎
> (256) 876-0326
>
> -----Original Message-----
> From: Fulvio Baccaglini [Caution-mailto:[hidden email]]
> Sent: Wednesday, September 12, 2018 5:33 AM
> To: CWE Research Discussion <[hidden email]>
> Subject: [Non-DoD Source] Re: CWE-365 "Race Condition in Switch" is
> not applicable to C/C++
 
> Hi Drew,
>
> Tracking those C11 paragraphs back to earlier C standards:
>
> ISO C99 has the same wording and paragraph numbers as ISO C11:
> [C99-6.8.4.2-5] and [C99-6.8.6.3-2]. See for instance draft N1256 from
> the ISO C WG14 standards page.
 
> ISO C90 also has the same wording, except that it uses the term
> "integral promotions" instead of "integer promotions" to express the same concept.
> Section references are:
 [C90-6.6.4.2] and [C90-6.6.6.3].
>
> Before ISO C there was the ANSI C standard (1989), and my
> understanding is that it can be considered equivalent to ISO C90.
 

> Fulvio
>
>
> On Tue, 2018-09-11 at 16:54 +0000, Buttner, Drew wrote:
>
> > Is there anyone else on the list that can confirm or shed some light
> > onto this issue?  The CWE was taken from CLASP back in the early
> > days of CWE.  Is it possible that this weakness affected earlier
> > versions of C?
> >
> > Thanks
> > Drew
> >
> >
> > -----Original Message-----
> > From: Fulvio Baccaglini <[hidden email]>
> > Sent: Friday, August 31, 2018 9:54 AM
> > To: CWE Research Discussion <[hidden email]>
> > Subject: CWE-365 "Race Condition in Switch" is not applicable to
> > C/C++
> >
> > The description of CWE-365 is "The code contains a switch statement
> > in which the switched variable can be modified while the switch is
> > still executing, resulting in unexpected behavior."
> >
> > This description, and the Demonstrative Example 1 for C, suggest
> > that the controlling expression of a switch statement can be
> > evaluated multiple times.
> >
> > However, the applicable paragraphs from the ISO C11 standard suggest
> > that it is evaluated only once:
> >
> > [C11-6.8.4.2-5] The switch statement - "The integer promotions are
> > performed on the controlling expression. The constant expression in
> > each case label is converted to the promoted type of the controlling
> > expression. If a converted value matches that of the promoted
> > controlling expression, control jumps to the statement following the
> > matched case label. Otherwise, if there is a default label, control
> > jumps to the labeled statement. If no converted case constant
> > expression matches and there is no default label, no part of the
> > switch body is executed."
> >
> > [C11-6.8.6.3] The break statement - "A break statement terminates
> > execution of the smallest enclosing switch or iteration statement."
> >
> > This is also the case for C++ (so that CWE-365 should not be part of
> > the
> > CWE-659 View: Weaknesses in Software Written in C++).
> >
> > (I am advised that also in Java and C# the controlling expression
> > would be evaluated only once).
> >
> > Fulvio
> >
> >
>
>
> CLASSIFICATION: UNCLASSIFIED



CLASSIFICATION: UNCLASSIFIED

smime.p7s (7K) Download Attachment