Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, 18 Jun 2020, Kees Cook wrote: > On Thu, Jun 18, 2020 at 09:56:18PM +0200, Julia Lawall wrote: > > @@ > > identifier i,fld; > > expression e; > > @@ > > > > \(\(i\|e.fld\|e->fld\) \& E\) > > > > The e will match all of the variants you are concerned about. > > Ah, I see! Okay, that's good. And the "& E" part is to effectively > collect it into E (as in, both the left and right of the & must match). Yes > So to do the matching from earlier: > > @@ > identifier i, fld; > expression e, ARG1, ARG2; > @@ > > array_size(\(\(i\|e.fld\|e->fld\) \& ARG1\), ARG2); Yes, that would be fine. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, Jun 18, 2020 at 09:56:18PM +0200, Julia Lawall wrote: > @@ > identifier i,fld; > expression e; > @@ > > \(\(i\|e.fld\|e->fld\) \& E\) > > The e will match all of the variants you are concerned about. Ah, I see! Okay, that's good. And the "& E" part is to effectively collect it into E (as in, both the left and right of the & must match). So to do the matching from earlier: @@ identifier i, fld; expression e, ARG1, ARG2; @@ array_size(\(\(i\|e.fld\|e->fld\) \& ARG1\), ARG2); ? -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] Fwd: [~npalix/ubuntu/coccinelle/focal] coccinelle 1.0.8~20.04npalix1 (Accepted)
On Thu, 18 Jun 2020, NICOLAS PALIX wrote: > Hi, > > I released the 1.0.8 version ported to 20.04 focal > on the coccinelle PPA. Thanks! julia > > Regards, > > __ > De: "Launchpad PPA" > À: "Nicolas Palix" > Envoyé: Jeudi 18 Juin 2020 20:55:16 > Objet: [~npalix/ubuntu/coccinelle/focal] coccinelle 1.0.8~20.04npalix1 > (Accepted) > > Accepted: > OK: coccinelle_1.0.8~20.04npalix1.tar.xz > OK: coccinelle_1.0.8~20.04npalix1.dsc > -> Component: main Section: devel > > coccinelle (1.0.8~20.04npalix1) focal; urgency=medium > > * New release 1.0.8 > > -- > https://launchpad.net/~npalix/+archive/ubuntu/coccinelle > You are receiving this email because you made this upload. > >___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] Fwd: [~npalix/ubuntu/coccinelle/focal] coccinelle 1.0.8~20.04npalix1 (Accepted)
Hi, I released the 1.0.8 version ported to 20.04 focal on the coccinelle PPA. Regards, De: "Launchpad PPA" À: "Nicolas Palix" Envoyé: Jeudi 18 Juin 2020 20:55:16 Objet: [~npalix/ubuntu/coccinelle/focal] coccinelle 1.0.8~20.04npalix1 (Accepted) Accepted: OK: coccinelle_1.0.8~20.04npalix1.tar.xz OK: coccinelle_1.0.8~20.04npalix1.dsc -> Component: main Section: devel coccinelle (1.0.8~20.04npalix1) focal; urgency=medium * New release 1.0.8 -- https://launchpad.net/~npalix/+archive/ubuntu/coccinelle You are receiving this email because you made this upload. ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Thu, 18 Jun 2020, Kees Cook wrote: > On Wed, Jun 17, 2020 at 08:54:03PM +0200, Julia Lawall wrote: > > > > > > On Wed, 17 Jun 2020, Kees Cook wrote: > > > > > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > > > > +@as@ > > > > +expression E1, E2; > > > > +@@ > > > > + > > > > +array_size(E1, E2) > > > > > > BTW, is there a way yet in Coccinelle to match a fully qualified (?) > > > identifier? For example, if I have two lines in C: > > > > > > A) > > > array_size(variable, 5); > > > B) > > > array_size(instance->member.size, 5); > > > C) > > > array_size(instance->member.size + 1, 5); > > > D) > > > array_size(function_call(variable), 5); > > > > > > > > > This matches A, B, C, and D: > > > > > > @@ > > > expression ARG1; > > > expression ARG2; > > > @@ > > > > > > array_size(ARG1, ARG2); > > > > > > > > > This matches only A: > > > > > > @@ > > > identifier ARG1; > > > expression ARG2; > > > @@ > > > > > > array_size(ARG1, ARG2); > > > > > > > > > How do I get something to match A and B but not C and D (i.e. I do not > > > want to match any operations, function calls, etc, only a variable, > > > which may be identified through dereference, array index, or struct > > > member access.) > > > > \(i\|e.fld\|e->fld\) > > > > would probably do what you want. It will also match cases where e is a > > function/macr call, but that is unlikely. > > > > If you want a single metavariable that contains the whole thing, you can > > have an expression metavariable E and then write: > > > > \(\(i\|e.fld\|e->fld\) \& E\) > > Can you give an example of how that would look for an @@ section? > > The problem I have is that I don't know the depth or combination of such > metavariables. There are a lot of combinations: > > a > a.b > a.b.c > a.b.c.d > a.b.c->d > a.b->c > a.b->c.d > a.b->c->d > a->b > a->b.c > a->b.c.d > a->b.c->d > a->b->c > a->b->c.d > a->b->c->d > ... @@ identifier i,fld; expression e; @@ \(\(i\|e.fld\|e->fld\) \& E\) The e will match all of the variants you are concerned about. julia > > > -- > Kees Cook > ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: misc: add array_size_dup script to detect missed overlow checks
On Wed, Jun 17, 2020 at 08:54:03PM +0200, Julia Lawall wrote: > > > On Wed, 17 Jun 2020, Kees Cook wrote: > > > On Mon, Jun 15, 2020 at 01:20:45PM +0300, Denis Efremov wrote: > > > +@as@ > > > +expression E1, E2; > > > +@@ > > > + > > > +array_size(E1, E2) > > > > BTW, is there a way yet in Coccinelle to match a fully qualified (?) > > identifier? For example, if I have two lines in C: > > > > A) > > array_size(variable, 5); > > B) > > array_size(instance->member.size, 5); > > C) > > array_size(instance->member.size + 1, 5); > > D) > > array_size(function_call(variable), 5); > > > > > > This matches A, B, C, and D: > > > > @@ > > expression ARG1; > > expression ARG2; > > @@ > > > > array_size(ARG1, ARG2); > > > > > > This matches only A: > > > > @@ > > identifier ARG1; > > expression ARG2; > > @@ > > > > array_size(ARG1, ARG2); > > > > > > How do I get something to match A and B but not C and D (i.e. I do not > > want to match any operations, function calls, etc, only a variable, > > which may be identified through dereference, array index, or struct > > member access.) > > \(i\|e.fld\|e->fld\) > > would probably do what you want. It will also match cases where e is a > function/macr call, but that is unlikely. > > If you want a single metavariable that contains the whole thing, you can > have an expression metavariable E and then write: > > \(\(i\|e.fld\|e->fld\) \& E\) Can you give an example of how that would look for an @@ section? The problem I have is that I don't know the depth or combination of such metavariables. There are a lot of combinations: a a.b a.b.c a.b.c.d a.b.c->d a.b->c a.b->c.d a.b->c->d a->b a->b.c a->b.c.d a->b.c->d a->b->c a->b->c.d a->b->c->d ... -- Kees Cook ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] moving variable declarations up
On Wed, 17 Jun 2020, Johannes Berg wrote: > On Wed, 2020-06-17 at 23:19 +0200, Johannes Berg wrote: > > On Wed, 2020-06-17 at 23:15 +0200, Julia Lawall wrote: > > > > +T x; > > > > > > Change the + to ++ > > Heh, something funny happened > > + u16 *foo; > + u16[2] bar; > > ... some code ... > > -u16 *foo; > -u16 *bar[2]; > > > What happened to the array index? > > I guess ... at some level I even understand it, that's a property of the > type, and I was moving the type around. But I really didn't expect that > :) If you get the latest version from github, everything should be ok. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] moving variable declarations up
On Wed, 17 Jun 2020, Johannes Berg wrote: > On Wed, 2020-06-17 at 23:19 +0200, Johannes Berg wrote: > > On Wed, 2020-06-17 at 23:15 +0200, Julia Lawall wrote: > > > > +T x; > > > > > > Change the + to ++ > > Heh, something funny happened > > + u16 *foo; > + u16[2] bar; > > ... some code ... > > -u16 *foo; > -u16 *bar[2]; > > > What happened to the array index? > > I guess ... at some level I even understand it, that's a property of the > type, and I was moving the type around. But I really didn't expect that > :) What version of Coccinelle do you have? There has been a lot of work on types recently that may have affected this. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH v2] coccinelle: misc: add array_size_dup script to detect missed overflow checks
Hi,
On 6/18/20 2:34 PM, Markus Elfring wrote:
> Why did you repeat a typo from the previous patch subject?
Where is the typo? I can't handle your suggestions because your mails constantly
break the threads. I just can't find them after due to missed/wrong In-Reply-To
headers. Again, this mail doesn't contain In-Reply-To header and highly likely I
will miss it when I will prepare next version of the patch.
>> +expression subE1 <= as.E1;
>> +expression subE2 <= as.E2;
>> +expression as.E1, as.E2, E3;
>
> How do you think about to use the following SmPL code variant?
>
> expression subE1 <= as.E1, subE2 <= as.E2, as.E1, as.E2, E3;
It's less readable and harder to review.
>
>> + when != \(\|\|\|\)
>
> I suggest to move the ampersand before the disjunction in such
> SmPL code exclusion specifications.
>
> + when != & \(E1 \| E2 \| subE1 \| subE2\)
Ok, I will fix this if there will be next version.
>
>
>> +coccilib.report.print_report(p2[0],
>> +f"WARNING: array_size is already used (line {p1[0].line}) to compute \
>> +the same size")
>
> I would prefer an other code formatting at such places.
>
> +coccilib.report.print_report(p2[0],
> + f"WARNING: array_size is already used (line
> {p1[0].line}) to compute the same size.")
>
No. It's pointless to break the line to save 5 chars this way.
I can use instead:
coccilib.report.print_report(p2[0], f"WARNING: array_size is already used (line
{p1[0].line}) to compute the same size")
or
msg = f"WARNING: array_size is already used (line {p1[0].line}) to compute the
same size"
coccilib.report.print_report(p2[0], msg)
or
coccilib.report.print_report(p2[0],
f"WARNING: array_size is already used (line {p1[0].line}) to compute the same
size")
And I prefer the last one if Julia will allow me to use more than 80 chars in
print string.
Thanks,
Denis
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] [PATCH v2] coccinelle: misc: add array_size_dup script to detect missed overlow checks
Detect an opencoded expression that is used before or after
array_size()/array3_size()/struct_size() to compute the same size.
Cc: Gustavo A. R. Silva
Cc: Kees Cook
Signed-off-by: Denis Efremov
---
Changes in v2:
- python rules moved next to SmPL patterns
- assignment operator used
- struct_size patterns fixed to check only E3, since
E1, E2 are sizeofs of a structure and a member
of a structure
scripts/coccinelle/misc/array_size_dup.cocci | 309 +++
1 file changed, 309 insertions(+)
create mode 100644 scripts/coccinelle/misc/array_size_dup.cocci
diff --git a/scripts/coccinelle/misc/array_size_dup.cocci
b/scripts/coccinelle/misc/array_size_dup.cocci
new file mode 100644
index ..c5214310285c
--- /dev/null
+++ b/scripts/coccinelle/misc/array_size_dup.cocci
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0-only
+///
+/// Check for array_size(), array3_size(), struct_size() duplicates.
+/// Three types of patterns for these functions:
+/// 1. An opencoded expression is used before array_size() to compute the
same size
+/// 2. An opencoded expression is used after array_size() to compute the same
size
+/// 3. Consecutive calls of array_size() with the same values
+/// From security point of view only first case is relevant. These functions
+/// perform arithmetic overflow check. Thus, if we use an opencoded expression
+/// before a call to the *_size() function we can miss an overflow.
+///
+// Confidence: High
+// Copyright: (C) 2020 Denis Efremov ISPRAS
+// Options: --no-includes --include-headers --no-loops
+
+virtual context
+virtual report
+virtual org
+
+@as@
+expression E1, E2;
+@@
+
+array_size(E1, E2)
+
+@as_next@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_next.p1;
+p2 << as_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as_prev@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* E1 * E2@p2
+
+@script:python depends on report@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as_prev.p1;
+p2 << as_prev.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: array_size is already used (line {p1[0].line}) to compute \
+the same size")
+
+@as_dup@
+expression subE1 <= as.E1;
+expression subE2 <= as.E2;
+expression as.E1, as.E2, E3;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array_size(E1, E2)@p1
+ ... when != \(E1\|E2\|subE1\|subE2\) aop E3
+ when != \(\|\|\|\)
+* array_size(E1, E2)@p2
+
+@script:python depends on report@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@script:python depends on org@
+p1 << as_dup.p1;
+p2 << as_dup.p2;
+@@
+
+coccilib.org.print_todo(p2[0],
+f"WARNING: same array_size (line {p1[0].line})")
+
+@as3@
+expression E1, E2, E3;
+@@
+
+array3_size(E1, E2, E3)
+
+@as3_next@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* E1 * E2 * E3@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(\|\|\|\|\|\)
+* array3_size(E1, E2, E3)@p2
+
+@script:python depends on report@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.report.print_report(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@script:python depends on org@
+p1 << as3_next.p1;
+p2 << as3_next.p2;
+@@
+
+coccilib.org.print_todo(p1[0],
+f"WARNING: array3_size is used down the code (line {p2[0].line}) to compute \
+the same size")
+
+@as3_prev@
+expression subE1 <= as3.E1;
+expression subE2 <= as3.E2;
+expression subE3 <= as3.E3;
+expression as3.E1, as3.E2, as3.E3, E4;
+assignment operator aop;
+position p1, p2;
+@@
+
+* array3_size(E1, E2, E3)@p1
+ ... when != \(E1\|E2\|E3\|subE1\|subE2\|subE3\) aop E4
+ when != \(\|\|\|\|\|\)
+* E1 * E2 * E3@p2
+
+@script:python depends on report@
+p1 << as3_prev.p1;
+p2 << as3_prev.p2;
+@@
+
+coccilib.report.print_report(p2[0],
+f"WARNING: array3_size is already used (line {p1[0].line}) to compute \
+the same size")
+
Re: [Cocci] moving variable declarations up
On Thu, 2020-06-18 at 07:59 +0200, Julia Lawall wrote: > > What's the difference between "..." and "... when any"? > > A ... B connects an A to the closest B, as you might like in some code > with lots of locks and unlocks. When any picks up any subsequent B, even > if there is another B between the matched A and B. OK, great, thanks for the explanation. Thanks a lot for your help! johannes ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] moving variable declarations up
On Thu, 2020-06-18 at 09:33 +0200, Markus Elfring wrote: > > > You tried it out to move the variable “bar” which is an array of two > > > pointers. > > > > Oops. I just mistyped that when transcribing it to email... It was 'u16 > > bar[2];' in the original. > > The intended absence of asterisks can make the clarification a bit easier. > Are there still any software development challenges to consider for > the movement of corresponding array sizes? Well, it's surprising that it went there? And it doesn't compile? johannes ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] moving variable declarations up
On Thu, 2020-06-18 at 09:19 +0200, Markus Elfring wrote: > > > > > +T x; > > > > > > > > Change the + to ++ > > > > Heh, something funny happened > > > > + u16 *foo; > > + u16[2] bar; > > > > ... some code ... > > > > -u16 *foo; > > -u16 *bar[2]; > > > > > > What happened to the array index? > > > > I guess ... at some level I even understand it, that's a property of the > > type, and I was moving the type around. But I really didn't expect that > > I find that there are further software development challenges to clarify. > > You tried it out to move the variable “bar” which is an array of two pointers. Oops. I just mistyped that when transcribing it to email... It was 'u16 bar[2];' in the original. johannes ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] moving variable declarations up
On Wed, 17 Jun 2020, Johannes Berg wrote: > On Wed, 2020-06-17 at 23:15 +0200, Julia Lawall wrote: > > > > > +T x; > > > > Change the + to ++ > > Hah, I think you mentioned that to me before, but I can never remember > what it does ... > > > There is no guarantee on the order in which the variables will appear. > > That's ok, I don't really care :) > > > > + > > > E; > > > ... > > > > Add when any after the ... > > What's the difference between "..." and "... when any"? A ... B connects an A to the closest B, as you might like in some code with lots of locks and unlocks. When any picks up any subsequent B, even if there is another B between the matched A and B. julia ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
