feat: I/Q/M symbolic tag browse for S7-1200 FW V4.5 via reconstructed preset dictionary#756
feat: I/Q/M symbolic tag browse for S7-1200 FW V4.5 via reconstructed preset dictionary#756tommasofaedo wants to merge 1 commit into
Conversation
gijzelaerr
left a comment
There was a problem hiding this comment.
Review Summary
This is a standalone example script (examples/browse_tags.py, 320 lines) that reconstructs Siemens' proprietary preset dictionary for zlib-decompressing I/Q/M EXPLORE blobs on S7-1200 FW V4.5 (V3 protocol). It does not touch the library code — no changes to s7/ or snap7/.
The technique (oracle-based dictionary reconstruction) is creative and well-documented. The PR description is thorough.
Issues
1. Hardcoded PLC IP address — PLC_HOST = '192.168.5.11' is baked in at the module level. This should be a CLI argument (sys.argv or argparse), not a constant.
2. unittest.mock to bypass auth — mock.patch.object(S7CommPlusConnection, '_post_auth_legitimation', ...) is a hack that reaches into library internals. If the connection API supports password='' or has a no-auth path, use that instead. If not, this should be documented more prominently as a workaround.
3. Accesses private APIs — conn._collect_explore_frames() and _build_explore_payload_v3() are internal methods. Since this is an example script (not library code), this is somewhat acceptable, but it means the script will break if those internals change.
4. No tests — no unit tests for _build_fdict(), _extract_tags(), or the regex parsing. Given the complexity of the oracle reconstruction, at least a test with a known input/output pair would be valuable.
5. The FDICT is incomplete — 594 of 32768 bytes. This works for the author's specific 40-tag project but may fail on PLCs with different tag configurations that reference unmapped dictionary positions. The script doesn't warn about this per-tag — it just produces ? characters silently.
6. PR description mentions changes to _s7commplus_client.py (adding browse_tags() method) but the actual diff is only the example script. The description is misleading — it shows code that isn't in the PR.
Security
✅ No malicious patterns. Standard zlib/regex. No network calls beyond the PLC connection.
PLC compatibility
✅ Example-only, doesn't touch library. No risk to other PLCs.
Verdict
Not ready to merge as-is:
- Hardcoded IP needs to be parameterized
- The mock.patch hack is fragile
- PR description claims library changes that aren't in the diff
- Needs at least basic tests for the extraction logic
As a research/example contribution it's interesting, but it needs cleanup before merging into examples/.
| import zlib | ||
| from unittest import mock | ||
|
|
||
| PLC_HOST = '192.168.5.11' |
There was a problem hiding this comment.
Hardcoded PLC_HOST = '192.168.5.11' — this should be a CLI argument. Users will always need to change this.
| conn = S7CommPlusConnection(host=PLC_HOST, port=PLC_PORT) | ||
| conn.connect(use_tls=False, password='', timeout=5.0) | ||
| try: | ||
| resp = conn.send_request(FunctionCode.EXPLORE, |
There was a problem hiding this comment.
mock.patch.object(S7CommPlusConnection, '_post_auth_legitimation', ...) — patching library internals in an example script is fragile. If the method is renamed or removed, this silently breaks. Is there a public API path for no-auth connections?
gijzelaerr
left a comment
There was a problem hiding this comment.
Thanks for the research — the oracle technique is clever and the results are impressive (33/40 tags recovered). Keeping this as an example script is the right approach given the limitations (firmware-specific FDICT, 6 permanent M-area gaps, silent breakage if Siemens changes the dictionary). It's not robust enough for a library API, but it's a useful reference for anyone working with V3 I/Q/M areas.
A few things to clean up before merging as an example:
- Remove the hardcoded IP (
192.168.5.11) — useargparseorsys.argvso it's actually runnable by others. - Add a module docstring explaining what this does, the firmware it was tested on, and the known limitations (M-area gaps, firmware-specific FDICT).
- Remove
mock.patch— the script patches internal methods which makes it fragile and confusing. If it needs specific connection setup, document the prerequisites instead. - Add a note that this is a proof-of-concept, not a supported library feature, and that the FDICT table is specific to S7-1200 FW V4.5 (Adler-32
0xce9b821b).
browse_tags.py
Tested on
Full_List_PLC_Tags.xlsx)Known limitation
The 6 M-area gap tags (Tag_5/11/16/18/20/22, all Byte or Word type) cannot
have their
LogicalAddressrecovered from the blob alone.ByteOffsetisalways correct. A hardcoded lookup table or a separate READ-based DataType
probe could resolve this, but both approaches are project-specific and are
not included in this patch.