[A][PATCH 0/1] CVE-2018-1068

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[A][PATCH 0/1] CVE-2018-1068

Khaled Elmously
CVE matrix shows this fix as needed for T/X/A/B/C. However the fix is already part of linux-stable 4.4.122 (so T/X have it) and linux-stable 4.15.10 (so B/C have it). So only needed in A.


Florian Westphal (1):
  netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets

 net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[A][PATCH 1/1] netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets

Khaled Elmously
From: Florian Westphal <[hidden email]>

CVE-2018-1068

We need to make sure the offsets are not out of range of the
total size.
Also check that they are in ascending order.

The WARN_ON triggered by syzkaller (it sets panic_on_warn) is
changed to also bail out, no point in continuing parsing.

Briefly tested with simple ruleset of
-A INPUT --limit 1/s' --log
plus jump to custom chains using 32bit ebtables binary.

Reported-by: <[hidden email]>
Signed-off-by: Florian Westphal <[hidden email]>
Signed-off-by: Pablo Neira Ayuso <[hidden email]>
(cherry-picked from b71812168571fa55e44cdd0254471331b9c4c4c6)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 9c6e619f452b..6890bb669197 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -2061,7 +2061,9 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
  if (match_kern)
  match_kern->match_size = ret;
 
- WARN_ON(type == EBT_COMPAT_TARGET && size_left);
+ if (WARN_ON(type == EBT_COMPAT_TARGET && size_left))
+ return -EINVAL;
+
  match32 = (struct compat_ebt_entry_mwt *) buf;
  }
 
@@ -2117,6 +2119,15 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
  *
  * offsets are relative to beginning of struct ebt_entry (i.e., 0).
  */
+ for (i = 0; i < 4 ; ++i) {
+ if (offsets[i] >= *total)
+ return -EINVAL;
+ if (i == 0)
+ continue;
+ if (offsets[i-1] > offsets[i])
+ return -EINVAL;
+ }
+
  for (i = 0, j = 1 ; j < 4 ; j++, i++) {
  struct compat_ebt_entry_mwt *match32;
  unsigned int size;
--
2.17.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [A][PATCH 0/1] CVE-2018-1068

Andy Whitcroft-3
In reply to this post by Khaled Elmously
On Mon, May 14, 2018 at 01:41:27AM -0400, Khalid Elmously wrote:

> CVE matrix shows this fix as needed for T/X/A/B/C. However the fix is
> already part of linux-stable 4.4.122 (so T/X have it) and linux-stable
> 4.15.10 (so B/C have it). So only needed in A.

Ok this indicated an autotriager flaw.  It is worth bringing these to my
attention sooner rather than later so they can be investigated.  I have
looked at the flaw and this is related to a new odd markup combination
coming from the security team where they are using upstream: and break-fix:
together (they are nominally mutually exclusive) and now using different
upstream repositories for linus' tree.  I have generalised the handling
for this situation and it is now resolved and the CVE matrix should
again show the truth.

I am slightly confused by your contention that T is not-affected as 4.4.122
has the fix; yes the hwe kernel is covered but the trusty GA kernel would
need separate handling.  The newly minted matrix output tends to confirm
this contention.

-apw

>
>
> Florian Westphal (1):
>   netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets
>
>  net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> --
> 2.17.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[Acked] [A][PATCH 1/1] netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets

Andy Whitcroft-3
In reply to this post by Khaled Elmously
On Mon, May 14, 2018 at 01:41:28AM -0400, Khalid Elmously wrote:

> From: Florian Westphal <[hidden email]>
>
> CVE-2018-1068
>
> We need to make sure the offsets are not out of range of the
> total size.
> Also check that they are in ascending order.
>
> The WARN_ON triggered by syzkaller (it sets panic_on_warn) is
> changed to also bail out, no point in continuing parsing.
>
> Briefly tested with simple ruleset of
> -A INPUT --limit 1/s' --log
> plus jump to custom chains using 32bit ebtables binary.
>
> Reported-by: <[hidden email]>
> Signed-off-by: Florian Westphal <[hidden email]>
> Signed-off-by: Pablo Neira Ayuso <[hidden email]>
> (cherry-picked from b71812168571fa55e44cdd0254471331b9c4c4c6)
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 9c6e619f452b..6890bb669197 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -2061,7 +2061,9 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
>   if (match_kern)
>   match_kern->match_size = ret;
>  
> - WARN_ON(type == EBT_COMPAT_TARGET && size_left);
> + if (WARN_ON(type == EBT_COMPAT_TARGET && size_left))
> + return -EINVAL;
> +
>   match32 = (struct compat_ebt_entry_mwt *) buf;
>   }
>  
> @@ -2117,6 +2119,15 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
>   *
>   * offsets are relative to beginning of struct ebt_entry (i.e., 0).
>   */
> + for (i = 0; i < 4 ; ++i) {
> + if (offsets[i] >= *total)
> + return -EINVAL;
> + if (i == 0)
> + continue;
> + if (offsets[i-1] > offsets[i])
> + return -EINVAL;
> + }
> +
>   for (i = 0, j = 1 ; j < 4 ; j++, i++) {
>   struct compat_ebt_entry_mwt *match32;
>   unsigned int size;
> --
> 2.17.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Looks to be sane to me.

Acked-by: Andy Whitcroft <[hidden email]>

-apw

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [A][PATCH 1/1] netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets

Kleber Souza
In reply to this post by Khaled Elmously
On 05/14/18 07:41, Khalid Elmously wrote:

> From: Florian Westphal <[hidden email]>
>
> CVE-2018-1068
>
> We need to make sure the offsets are not out of range of the
> total size.
> Also check that they are in ascending order.
>
> The WARN_ON triggered by syzkaller (it sets panic_on_warn) is
> changed to also bail out, no point in continuing parsing.
>
> Briefly tested with simple ruleset of
> -A INPUT --limit 1/s' --log
> plus jump to custom chains using 32bit ebtables binary.
>
> Reported-by: <[hidden email]>
> Signed-off-by: Florian Westphal <[hidden email]>
> Signed-off-by: Pablo Neira Ayuso <[hidden email]>
> (cherry-picked from b71812168571fa55e44cdd0254471331b9c4c4c6)
> Signed-off-by: Khalid Elmously <[hidden email]>

Clean cherry-pick:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


As Andy mentioned on the other email, by the CVE matrix it seems that
trusty/linux needs the fix as well.


Thanks,
Kleber


> ---
>  net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 9c6e619f452b..6890bb669197 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -2061,7 +2061,9 @@ static int ebt_size_mwt(struct compat_ebt_entry_mwt *match32,
>   if (match_kern)
>   match_kern->match_size = ret;
>  
> - WARN_ON(type == EBT_COMPAT_TARGET && size_left);
> + if (WARN_ON(type == EBT_COMPAT_TARGET && size_left))
> + return -EINVAL;
> +
>   match32 = (struct compat_ebt_entry_mwt *) buf;
>   }
>  
> @@ -2117,6 +2119,15 @@ static int size_entry_mwt(struct ebt_entry *entry, const unsigned char *base,
>   *
>   * offsets are relative to beginning of struct ebt_entry (i.e., 0).
>   */
> + for (i = 0; i < 4 ; ++i) {
> + if (offsets[i] >= *total)
> + return -EINVAL;
> + if (i == 0)
> + continue;
> + if (offsets[i-1] > offsets[i])
> + return -EINVAL;
> + }
> +
>   for (i = 0, j = 1 ; j < 4 ; j++, i++) {
>   struct compat_ebt_entry_mwt *match32;
>   unsigned int size;
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [T/A][PATCH 0/1] CVE-2018-1068

Khaled Elmously
In reply to this post by Andy Whitcroft-3
On 2018-05-14 10:40:10 , Andy Whitcroft wrote:

> On Mon, May 14, 2018 at 01:41:27AM -0400, Khalid Elmously wrote:
>
> > CVE matrix shows this fix as needed for T/X/A/B/C. However the fix is
> > already part of linux-stable 4.4.122 (so T/X have it) and linux-stable
> > 4.15.10 (so B/C have it). So only needed in A.
>
> Ok this indicated an autotriager flaw.  It is worth bringing these to my
> attention sooner rather than later so they can be investigated.  I have
> looked at the flaw and this is related to a new odd markup combination
> coming from the security team where they are using upstream: and break-fix:
> together (they are nominally mutually exclusive) and now using different
> upstream repositories for linus' tree.  I have generalised the handling
> for this situation and it is now resolved and the CVE matrix should
> again show the truth.
>
> I am slightly confused by your contention that T is not-affected as 4.4.122
> has the fix; yes the hwe kernel is covered but the trusty GA kernel would
> need separate handling.  The newly minted matrix output tends to confirm
> this contention.

Yes I was thinking of the 4.4 trusty kernels for some reason. You're right, GA trusty needs this fix too, and it applies cleanly there.

Thanks for the feedback.


>
> -apw
>
> >
> >
> > Florian Westphal (1):
> >   netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets
> >
> >  net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > --
> > 2.17.0
> >
> >
> > --
> > kernel-team mailing list
> > [hidden email]
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[Trusty/Artful]: [A][PATCH 0/1] CVE-2018-1068

Kleber Souza
In reply to this post by Khaled Elmously
On 05/14/18 07:41, Khalid Elmously wrote:
> CVE matrix shows this fix as needed for T/X/A/B/C. However the fix is already part of linux-stable 4.4.122 (so T/X have it) and linux-stable 4.15.10 (so B/C have it). So only needed in A.
>
>
> Florian Westphal (1):
>   netfilter: ebtables: CONFIG_COMPAT: don't trust userland offsets
>
>  net/bridge/netfilter/ebtables.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>

Applied to trusty/master-next and artful/master-next branches.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team