Skip to content

Commit

Permalink
Merge pull request #413 from GoSecure/avoid-leaking-fd
Browse files Browse the repository at this point in the history
Avoid leaking file descriptors for terminated sessions
  • Loading branch information
obilodeau authored Oct 27, 2022
2 parents 9b2290a + 0d38657 commit 2f20e41
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ For a detailed view of what has changed, refer to the {uri-repo}/commits/master[
* Fixed `pyrdp-player` on macOS platforms ({uri-issue}362[#362])
* Fixed `pyrdp-convert` pcap processing when victim IP and MITM IP are the same ({uri-issue}366[#366])
* Fixed NLA redirection problems if original target and NLA redirection target are the same ({uri-issue}342[#342], {uri-issue}343[#343])
* Fixed leak of file descriptors due to missing close on replay file recording ({uri-issue}392[#392], {uri-issue}413[#413])
* Added a missing dependency for the GUI on Ubuntu 20.04 LTS ({uri-issue}348[#348], {uri-issue}351[#351], {uri-issue}355[#355])
* No longer assuming every connection will have VirtualChannels ({uri-issue}375[#375])
* Some minor protocol-level fixes ({uri-issue}408[#408])
Expand Down
1 change: 1 addition & 0 deletions pyrdp/mitm/TCPMITM.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ def onClientDisconnection(self, reason):
if self.recorder.recordFilename:
self.statCounter.logReport(self.log, {"replayFilename":
self.recorder.recordFilename})
self.recorder.finalize()
else:
self.statCounter.logReport(self.log)

Expand Down
13 changes: 13 additions & 0 deletions pyrdp/recording/recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def __init__(self, transports: List[LayerChainItem]):
}

self.topLayers = []
self.openFileLayers = []
self.recordFilename = None

for transport in transports:
Expand All @@ -52,6 +53,9 @@ def addTransport(self, transportLayer: LayerChainItem):
player.setPrevious(transportLayer)
self.topLayers.append(player)

if isinstance(transportLayer, FileLayer):
self.openFileLayers.append(transportLayer)

def setParser(self, messageType: PlayerPDUType, parser: Parser):
"""
Set the parser to use for a given message type.
Expand Down Expand Up @@ -81,6 +85,11 @@ def record(self, pdu: Optional[PDU], messageType: PlayerPDUType):
def getCurrentTimeStamp(self) -> int:
return PlayerLayer.timeStampFunction()

def finalize(self):
"""When the recording is finished"""
for layer in self.openFileLayers:
layer.close()


class FileLayer(LayerChainItem):
"""
Expand Down Expand Up @@ -114,3 +123,7 @@ def sendBytes(self, data: bytes):
self.fd.write(data)
else:
log.error("Recording file handle closed, cannot write message: %(message)s", {"message": data})

def close(self):
if self.fd:
self.fd.close()

0 comments on commit 2f20e41

Please sign in to comment.