Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed with convert .pyrdp to .mp4: Segmentation fault in GDI Glyph #428

Closed
yangyang5214 opened this issue Dec 15, 2022 · 5 comments Β· Fixed by #429
Closed

Failed with convert .pyrdp to .mp4: Segmentation fault in GDI Glyph #428

yangyang5214 opened this issue Dec 15, 2022 · 5 comments Β· Fixed by #429
Assignees
Labels
bug Something isn't working
Milestone

Comments

@yangyang5214
Copy link

demo.zip

➜  ~ zip -sf demo.zip 
Archive contains:
  tmp.pyrdp
Total 1 entries (2960418 bytes)

Env: win7

πŸ‘† pyrdp file(in demo.zip ) convert to mp4 file failed (may be python process breakdown).

  • running log
# docker run --rm --user root -v $PWD:/tmp  hp_pyrdp /bin/bash -c 'pyrdp-convert.py -o /tmp -f mp4 /tmp/tmp.pyrdp'
  0% (0 of 285) |                        | Elapsed Time: 0:00:00 ETA:  --:--:--
  1% (4 of 285) |                        | Elapsed Time: 0:00:00 ETA:   0:00:31
@obilodeau
Copy link
Member

I get a segmentation fault with your pyrdp file on my Python 3.10 Linux environment at the same location:

$ pyrdp-convert.py -f mp4 tmp.pyrdp 
[*] Converting 'tmp.pyrdp' to MP4
  1% (4 of 285) |#                                           | Elapsed Time: 0:00:00 ETA:   0:00:23
Segmentation fault (core dumped)

Using the built-in faulthandler module we can get a little more context around the crash:

$ python -q -X faulthandler ~/gosecure/src/pyrdp/bin/pyrdp-convert.py -f mp4 tmp.pyrdp
[*] Converting 'tmp.pyrdp' to MP4
  1% (4 of 285) |#                                                                                     | Elapsed Time: 0:00:00 ETA:   0:00:22Fatal Python error: Segmentation fault

Current thread 0x00007f1a4d444740 (most recent call first):
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/player/gdi/cache.py", line 149 in __init__
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/player/gdi/draw.py", line 386 in fastGlyph
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/parser/rdp/orders/parse.py", line 169 in _parse_fast_glyph
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/parser/rdp/orders/parse.py", line 93 in _parse_primary
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/parser/rdp/orders/parse.py", line 83 in _parse_order
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/parser/rdp/orders/parse.py", line 69 in parse
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/player/RenderingEventHandler.py", line 49 in onFastPathOutput
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/player/BaseEventHandler.py", line 144 in onFastPathFragment
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/player/BaseEventHandler.py", line 73 in onPDUReceived
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/convert/MP4EventHandler.py", line 74 in onPDUReceived
  File "/home/olivier/Documents/gosecure/src/pyrdp/pyrdp/convert/ReplayConverter.py", line 30 in process
  File "/home/olivier/gosecure/src/pyrdp/bin/pyrdp-convert.py", line 95 in <module>

Extension modules: _cffi_backend, zope.interface._zope_interface_coptimizations, numpy.core._multiarray_umath, numpy.core._multiarray_tests, numpy.linalg._umath_linalg, numpy.fft._pocketfft_internal, numpy.random._common, numpy.random.bit_generator, numpy.random._bounded_integers, numpy.random._mt19937, numpy.random.mtrand, numpy.random._philox, numpy.random._pcg64, numpy.random._sfc64, numpy.random._generator, xxsubtype, shiboken2.shiboken2, PySide2.QtCore, PySide2.QtGui, PySide2.QtWidgets, rle, av._core, av.logging, av.bytesource, av.buffer, av.audio.format, av.enum, av.error, av.utils, av.option, av.descriptor, av.dictionary, av.format, av.stream, av.container.streams, av.sidedata.motionvectors, av.sidedata.sidedata, av.packet, av.container.input, av.container.output, av.container.pyio, av.container.core, av.codec.context, av.video.format, av.video.reformatter, av.plane, av.video.plane, av.video.frame, av.video.stream, av.codec.codec, av.frame, av.audio.layout, av.audio.plane, av.audio.frame, av.audio.stream, av.audio.fifo, av.audio.resampler, av.video.codeccontext (total: 58)
Segmentation fault (core dumped)

I think we pass data that is too large to the QBitmap.fromdata() call in pyrdp/player/gdi/cache.py, line 149. Maybe because that glyph type isn't supported or properly recognized.

I tried on another capture file and I'm not getting a hit on that line of code however...

Your server is Windows 7 and do you have the logs for when you recorded that session? I am looking for warning-level log lines that says "One or more unimplemented drawing orders called!". If I know which one I could implement it.

@obilodeau obilodeau added this to the v1.3.0 milestone Dec 19, 2022
@obilodeau obilodeau added bug Something isn't working help wanted Extra attention is needed question Further information is requested labels Dec 19, 2022
@obilodeau obilodeau changed the title Failed with convert .pyrdp to .mp4 Failed with convert .pyrdp to .mp4: Segmentation fault in GDI Glyph Dec 19, 2022
@yangyang5214
Copy link
Author

I can't find "One or more unimplemented drawing orders called!" log, when set -L DEBUG

eg:

python /pyrdp/bin/pyrdp-mitm.py 55.0.0.13 -L DEBUG

@obilodeau
Copy link
Member

This patch should do the trick:

diff --git a/bin/pyrdp-convert.py b/bin/pyrdp-convert.py
index b87b80b..0ffd3d4 100755
--- a/bin/pyrdp-convert.py
+++ b/bin/pyrdp-convert.py
@@ -63,6 +63,10 @@ if __name__ == "__main__":
     if not HAS_GUI and args.format == "mp4":
         sys.stderr.write("Error: MP4 conversion requires the full PyRDP installation.")
         sys.exit(1)
+    elif HAS_GUI and args.format == "mp4":
+        # Initialize QT because QPixmap relies on it
+        from PySide2.QtWidgets import QApplication
+        app = QApplication()
 
     logging.basicConfig(level=logging.CRITICAL)
     logging.getLogger("scapy").setLevel(logging.ERROR)
diff --git a/pyrdp/player/gdi/cache.py b/pyrdp/player/gdi/cache.py
index 34ff73b..7cb6e90 100644
--- a/pyrdp/player/gdi/cache.py
+++ b/pyrdp/player/gdi/cache.py
@@ -10,7 +10,7 @@ GDI Cache Management Layer.
 
 from typing import List
 from PySide2.QtCore import QSize
-from PySide2.QtGui import QBrush, QImage, QBitmap
+from PySide2.QtGui import QBrush, QImage, QBitmap, QPixmap
 
 from pyrdp.parser.rdp.orders.common import Glyph
 
@@ -146,7 +146,10 @@ class GlyphEntry:
         self.w = glyph.w
         self.h = glyph.h
 
-        self.bitmap = QBitmap.fromData(QSize(self.w, self.h), glyph.data, QImage.Format_Mono)
+        #import ipdb; ipdb.set_trace()
+        i = QImage(glyph.data, self.w, self.h, QImage.Format_Mono)
+        self.bitmap = QPixmap.fromImageInPlace(i)
+        #self.bitmap = QBitmap.fromData(QSize(self.w, self.h), glyph.data, QImage.Format_Mono)
 
 
 class GlyphCache:

With this patch I was able to convert your pyrdp file successfully and I confirmed that it didn't slow down other conversion jobs. I will commit a slightly cleaner version tomorrow.

@obilodeau obilodeau removed help wanted Extra attention is needed question Further information is requested labels Dec 20, 2022
@obilodeau obilodeau modified the milestones: v1.3.0, v1.2.0 Dec 20, 2022
@obilodeau obilodeau self-assigned this Dec 20, 2022
@obilodeau
Copy link
Member

The fix is not correct. It no longer crashes but the video is corrupted. Check a screenshot of the converted video:

image

Here is a screenshot of pyrdp-player with the patch applied:

image

and here without the patch applied (notice that it doesn't segfault like pyrdp-convert does):

image

Somehow the fix introduces visual corruption.

@obilodeau
Copy link
Member

Turns out that only the QT initialization part was required to avoid the segfault. I can keep using QBitmap.fromData() and there is no visual corruption. Will submit a fix today.

obilodeau added a commit that referenced this issue Dec 20, 2022
It's a weird fix but other attempts lead to video corruption see ticket for details.
obilodeau added a commit that referenced this issue Dec 20, 2022
obilodeau added a commit that referenced this issue Dec 20, 2022
It's a weird fix but other attempts lead to video corruption see ticket for details.

Tests started failing in CI so the QT_QPA_PLATFORM environment variable trick was required to avoid that.
obilodeau added a commit that referenced this issue Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants