Bug 204005 - [patch][pf] PF_ANEQ macro improperly compare IPv4 packets.
Summary: [patch][pf] PF_ANEQ macro improperly compare IPv4 packets.
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-10-24 17:17 UTC by Miłosz Kaniewski
Modified: 2016-08-17 15:15 UTC (History)
1 user (show)

See Also:


Attachments
patch (619 bytes, patch)
2015-10-24 17:17 UTC, Miłosz Kaniewski
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miłosz Kaniewski 2015-10-24 17:17:18 UTC
Created attachment 162429 [details]
patch

PF_ANEQ() macro will in most situations returns TRUE comparing two identical IPv4 packets (when it should return FALSE). It happens because PF_ANEQ() doesn't stop if first 32 bits of IPv4 packets are equal and starts to check next 3*32 bits (like for IPv6 packet). Those bits containt some garbage and in result PF_ANEQ() wrongly returns TRUE.

Fix: Check if packet is of AF_INET type and if it is then compare only first 32 bits of data.
Proposed fix in attachment.

This bug was already described and repaired in OpenBSD pf:
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/pfvar.h?f=h#rev1.287
Comment 1 commit-hook freebsd_committer freebsd_triage 2015-10-25 13:15:01 UTC
A commit references this bug:

Author: kp
Date: Sun Oct 25 13:14:54 UTC 2015
New revision: 289932
URL: https://svnweb.freebsd.org/changeset/base/289932

Log:
  PF_ANEQ() macro will in most situations returns TRUE comparing two identical
  IPv4 packets (when it should return FALSE). It happens because PF_ANEQ() doesn't
  stop if first 32 bits of IPv4 packets are equal and starts to check next 3*32
  bits (like for IPv6 packet). Those bits containt some garbage and in result
  PF_ANEQ() wrongly returns TRUE.

  Fix: Check if packet is of AF_INET type and if it is then compare only first 32
  bits of data.

  PR:		204005
  Submitted by:	Mi?osz Kaniewski

Changes:
  head/sys/net/pfvar.h
Comment 2 Kristof Provost freebsd_committer freebsd_triage 2015-10-25 13:18:28 UTC
Thanks!

It looks like this was mostly relatively harmless (it's most often used to optimise things, i.e. don't do anything if the address doesn't change anyway), but it's good to fix this anyway.

You're clearly right that this was wrong and needed to be fixed.
Comment 3 commit-hook freebsd_committer freebsd_triage 2016-08-17 15:15:07 UTC
A commit references this bug:

Author: kp
Date: Wed Aug 17 15:14:21 UTC 2016
New revision: 304293
URL: https://svnweb.freebsd.org/changeset/base/304293

Log:
  MFC r289932, r289940:

  PF_ANEQ() macro will in most situations returns TRUE comparing two identical
  IPv4 packets (when it should return FALSE). It happens because PF_ANEQ() doesn't
  stop if first 32 bits of IPv4 packets are equal and starts to check next 3*32
  bits (like for IPv6 packet). Those bits containt some garbage and in result
  PF_ANEQ() wrongly returns TRUE.

  Fix: Check if packet is of AF_INET type and if it is then compare only first 32
  bits of data.

  PR:             204005
  Submitted by:   Mi?osz Kaniewski

Changes:
_U  stable/10/
  stable/10/sys/net/pfvar.h