-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add DICOM (Digital Imaging and Communications in Medicine) protocol support #4891
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4891 +/- ##
==========================================
- Coverage 80.84% 80.63% -0.22%
==========================================
Files 369 370 +1
Lines 90963 91662 +699
==========================================
+ Hits 73539 73908 +369
- Misses 17424 17754 +330
🚀 New features to boost your workflow:
|
|
Thanks for the PR. I’ve stared the unit tests. Looks good, however, could you please add type hints to your layer? |
Added |
|
Besides the small Flake8 issues, LGTM. @gpotter2 do you want to have a final look? |
|
@polybassa fixed the Flake8 issues |
|
I trust you @polybassa ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive support for the DICOM (Digital Imaging and Communications in Medicine) Upper Layer Protocol to Scapy, enabling network analysis and interaction with medical imaging systems. The implementation follows the DICOM PS3.8 specification and includes extensive packet definitions for protocol negotiation, data transfer, and DIMSE commands.
Changes:
- Added complete DICOM protocol layer implementation with PDU types, variable items, and DIMSE command packets
- Implemented a high-level DICOMSocket class for association management and DIMSE operations
- Added 59 comprehensive unit tests covering packet construction, parsing, and round-trip serialization
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| scapy/contrib/dicom.py | Main implementation file containing DICOM packet classes, field types, PDU definitions, DIMSE commands, and DICOMSocket for application-level operations |
| test/contrib/dicom.uts | Comprehensive test suite with 59 tests covering module loading, PDU construction/parsing, variable items, DIMSE packets, helper functions, and edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| value_len = struct.unpack( | ||
| "<I", dimse_bytes[offset + 4:offset + 8] | ||
| )[0] | ||
| if tag_group == 0x0000 and tag_elem == 0x0900 and value_len == 2: |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the parse_dimse_status function, after checking that offset + 8 is within bounds, the code reads the value at offset + 8 to offset + 10 without verifying that offset + 10 is also within bounds. If value_len is 2 but there are fewer than 2 bytes remaining after offset + 8, this could cause an IndexError or read beyond the buffer.
| if tag_group == 0x0000 and tag_elem == 0x0900 and value_len == 2: | |
| if ( | |
| tag_group == 0x0000 | |
| and tag_elem == 0x0900 | |
| and value_len == 2 | |
| ): | |
| # Ensure there are at least 2 bytes available for the status | |
| if offset + 10 > len(dimse_bytes) or offset + 10 > group_end_offset: | |
| break |
| def getfield(self, pkt: Optional[Packet], s: bytes) -> Tuple[bytes, bytes]: | ||
| if len(s) < 8: | ||
| return s, b"" | ||
| tag_g, tag_e, length = struct.unpack("<HHI", s[:8]) |
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The getfield method in DICOMElementField reads a length value from the packet but doesn't validate that sufficient bytes are available before slicing s[8:8 + length]. If length is very large or the packet is truncated, this could read beyond the available data without raising an error, potentially causing silent data corruption or unexpected behavior.
| tag_g, tag_e, length = struct.unpack("<HHI", s[:8]) | |
| tag_g, tag_e, length = struct.unpack("<HHI", s[:8]) | |
| # Ensure the buffer contains the full value as declared by the length | |
| if len(s) < 8 + length: | |
| raise Scapy_Exception( | |
| "Not enough bytes to decode DICOM element value: " | |
| f"expected {length} bytes, only {len(s) - 8} available" | |
| ) |
Summary
DICOM communications is used in medical imaging systems. This PR adds Scapy layers for the DICOM Upper Layer Protocol (DICOM PS3.8).
Features
Usage
Testing
test/contrib/dicom.utsReferences