Project

General

Profile

Actions
Copy link

Bug #5443

closed

ftp-data: failed assertion

Added by Philippe Antoine over 2 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Victor Julien
Target version:
7.0.1
Affected Versions:
Effort:
Difficulty:
Label:

Description

Found by oss-fuzz
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49010

./src/suricata -k none -c suricata.yaml --set stream.midstream=true --runmode=single -r lol.pcap

Victor, is this assertion really meant to be unreachable ?

Files

lol.pcap (4.13 KB) lol.pcap Philippe Antoine, 07/15/2022 01:46 PM

Related issues 1 (0 open — 1 closed)

Related to Suricata - Bug #5205: FTP-data unrecognized depending on multi-threadingClosedPhilippe AntoineActions
Actions
Copy link
 #2

Updated by Philippe Antoine over 2 years ago

  • Related to Bug #5205: FTP-data unrecognized depending on multi-threading added
Actions
Copy link
 #3

Updated by Philippe Antoine over 2 years ago

Took me some time to figure out that I was needing --runmode=single to reproduce due to #5205 (fuzz target runs in single thread and so ftp-data detection goes alright)

Actions
Copy link
 #4

Updated by Victor Julien over 2 years ago

  • Status changed from New to Assigned
  • Priority changed from Normal to High
Actions
Copy link
 #5

Updated by Philippe Antoine about 2 years ago

My analysis :
- we have a FTP flow which expects a ftp-data flow
- we use stream.midstream=true
- the ftp-data flow has a first packet with data
- the ftp-data flow has then a second packet in same direction with RST flag (and no ACK flag)
- RST triggers the parse of all available data, even if it has never been acked

Should we process any of these data that has never been acked ?

Actions
Copy link
 #6

Updated by Victor Julien about 2 years ago

  • Target version changed from 7.0.0-beta1 to 8.0.0-beta1
Actions
Copy link
 #7

Updated by Philippe Antoine about 2 years ago

Why not 7.0.rc1 ?

Actions
Copy link
 #8

Updated by Victor Julien about 2 years ago

  • Target version changed from 8.0.0-beta1 to 7.0.0-rc1

Accident during mass retargeting, thanks for catching this.

Actions
Copy link
 #9

Updated by Philippe Antoine almost 2 years ago

I would disable the debug assertion if we cannot have a better fix quickly.
This would allow to have fuzzing going on and we would still have the issue to investigate...

Actions
Copy link
 #10

Updated by Jeff Lucovsky almost 2 years ago

  • Status changed from Assigned to Closed
Actions
Copy link
 #11

Updated by Victor Julien almost 2 years ago

  • Private changed from Yes to No
Actions
Copy link
 #12

Updated by Philippe Antoine almost 2 years ago

  • Status changed from Closed to Assigned

The bug is still there to be fixed, so reopening

Actions
Copy link
 #14

Updated by Philippe Antoine almost 2 years ago

  • Priority changed from High to Normal
Actions
Copy link
 #15

Updated by Victor Julien almost 2 years ago

  • Target version changed from 7.0.0-rc1 to 7.0.0-rc2
Actions
Copy link
 #16

Updated by Victor Julien over 1 year ago

  • Target version changed from 7.0.0-rc2 to 7.0.0
Actions
Copy link
 #17

Updated by Victor Julien over 1 year ago

  • Priority changed from Normal to High
Actions
Copy link
 #18

Updated by Victor Julien over 1 year ago

  • Target version changed from 7.0.0 to 7.0.1
Actions
Copy link
 #19

Updated by Victor Julien over 1 year ago

  • Status changed from Assigned to In Review
Actions
Copy link
 #20

Updated by Victor Julien about 1 year ago

  • Status changed from In Review to Closed
  • Priority changed from High to Normal

https://github.com/OISF/suricata/pull/9320

Issue was caused by data on RST feeding data to app-layer after EOF was already sent to app-layer. Data on RST tracked in #6244.

Actions
Copy link

Also available in: Atom PDF