Skip to content

[c166] Fix overflow#6599

Open
SSharshunov wants to merge 4 commits into
rizinorg:devfrom
SSharshunov:c166_fix_leaks
Open

[c166] Fix overflow#6599
SSharshunov wants to merge 4 commits into
rizinorg:devfrom
SSharshunov:c166_fix_leaks

Conversation

@SSharshunov

@SSharshunov SSharshunov commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Your checklist for this pull request

  • I've read the guidelines for contributing to this repository.
  • I made sure to follow the project's coding style.
  • I've documented every RZ_API function and struct this PR changes.
  • I've added tests that prove my changes are effective (required for changes to RZ_API).
  • I've updated the Rizin book with the relevant information (if needed).
  • I've used AI tools to generate fully or partially these code changes and I'm sure the changes are not copyrighted by somebody else.

Detailed description

START REPORT

Heap out-of-bounds read in Rizin OMF166 loader (load_omf166_global_sym_record)

Summary

The OMF166 object-file loader reads a symbol-name length byte from the file and
adds it to the record cursor without checking it against the buffer size. A
crafted .obj with a LOCSYM/PUBDEF/GLBDEF/DEBSYM record whose name
length is large drives the cursor past the end of the mapped file buffer; the
next fixed-field reads (rz_read_le16_offset / rz_read_le8_offset) then read
out of bounds on the heap.

The file is auto-detected as OMF166 by rz_bin (no plugin flag needed), so the
overflow is reached by simply opening a malicious file, e.g. rz-bin file.obj
or rizin file.obj. A 22-byte input is enough.

  • Component: librz/bin/format/omf/omf166.c, load_omf166_global_sym_record()
  • Class: CWE-125 (out-of-bounds read), heap
  • Impact: crash on file open (DoS); the out-of-bounds bytes are parsed into
    symbol fields, so adjacent heap memory can influence loader output
    (information disclosure)
  • Version: current master

...

Test plan

...

Closing issues

...

@notxvilka notxvilka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make PR description shorter

Also it breaks one of the tests:

[XX] 00h00m03s265ms db/analysis/c166 C166 measure binary file iA
RZ_COLOR=0 RZ_UTF8=0 RZ_NOPLUGINS=1 /usr/bin/rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -eflirt.sigdb.load.system=false -esearch.show_progress=false -eflirt.sigdb.load.home=false -N -qc 'e asm.arch=c166
e asm.bytes=true
iA
echo
iS
echo
aaa
pd 150
echo
pd 21 @ 0x00c001e6
echo
pd @ 0x00c018da
echo
pdf @ 0x00c013b0
echo
pdf @ 0x00c0168a
' bins/omf/omf166/measure
-- stdout
--- expected
+++ actual
@@ -56,20 +56,20 @@
        @|   ;-- section.PR_GETKEY_FCODE:
        @|   ;-- _getkey:
 / _getkey();
-|      @=-> 0x00c0001c      9ab7fe70       jnb   0xff6e, _getkey       ; Getkey.c:22 ; 0xc0001c ; [09] -rw- section size 12 named PR_GETKEY_FCODE
+|      @=-> 0x00c0001c      9ab7fe70       jnb   S0RIC.7, _getkey      ; Getkey.c:22 ; 0xc0001c ; [09] -rw- section size 12 named PR_GETKEY_FCODE
 |       |   0x00c00020      f2f4b2fe       mov   r4, 0xfe06:0x3eb2     ; Getkey.c:26
-|       |   0x00c00024      7eb7           bclr  0xff6e                ; Getkey.c:27
+|       |   0x00c00024      7eb7           bclr  S0RIC.7               ; Getkey.c:27
 \       |   0x00c00026      db00           rets                        ; Getkey.c:28
 / sym.isr_vec_0x28();
 \      ,==< 0x00c00028      fac07a00       jmps  Class_B_trap          ; 0xc0007a ; "\r\xff\xff\xff\xff\xff\xfa\xc0\x80\U00000012\xa5Z\xa5\xa5р\xe6\xea"
        ||   ;-- section.C_CLRMEMSEC_UNKNOWN:
-       ||   0x00c0002c      1a800400       bfldh 0xff00, #0x00, #0x04  ; [26] -r-- section size 24 named C_CLRMEMSEC_UNKNOWN
+       ||   0x00c0002c      1a800400       bfldh P0L, #0x00, #0x04     ; [26] -r-- section size 24 named C_CLRMEMSEC_UNKNOWN
        ||   0x00c00030      40c2           cmp   r12, r2
-       ||   0x00c00032      0fa0           bset  0xff40.0
-       ||   0x00c00034      04005cc2       add   0xfe06:0x025c, 0xfe00
-       ||   0x00c00038      03000000       addb  0xfe00, 0xfe00:0x0000
+       ||   0x00c00032      0fa0           bset  T2CON.0
+       ||   0x00c00034      04005cc2       add   0xfe06:0x025c, DPP0
+       ||   0x00c00038      03000000       addb  DPP0, 0xfe00:0x0000
        ||   0x00c0003c      0080           add   r8, r0
-       ||   0x00c0003e      04005bc2       add   0xfe06:0x025b, 0xfe00
+       ||   0x00c0003e      04005bc2       add   0xfe06:0x025b, DPP0
        ||   0x00c00042      0000           add   r0, r0
        ||   ;-- section.C_LIB_NCONST_NCONST:
        ||   0x00c00044      0000           add   r0, r0                ; [19] -r-- section size 48 named C_LIB_NCONST_NCONST
@@ -83,13 +83,13 @@
        ||   0x00c00056      1c46           rol   r6, #0x04
        ||   0x00c00058      0050           add   r5, r0
        ||   0x00c0005a      c3470024       CoSTORE r4, 0xffde
-       ||   0x00c0005e      74498096       or    0xfe04:0x1680, 0xfe92
+       ||   0x00c0005e      74498096       or    0xfe04:0x1680, CC9
        ||   0x00c00062      184b           addc  r4, [r3]
        ||   0x00c00064      20bc           sub   r11, r12
        ||   0x00c00066      be4c           bclr  0xfd98.11
        ||   0x00c00068      ca1b0e5a       calla- cc_NUSR1, 0xc05a0e   ; 0x5a0e
        ||   0x00c0006c      1cc2           rol   r2, #0x0c
-       ||   0x00c0006e      5367aec5       xorb  0xfece, 0xfe06:0x05ae
+       ||   0x00c0006e      5367aec5       xorb  PECC7, 0xfe06:0x05ae
        ||   0x00c00072      9d74           jmpr  cc_NC/UGE, 0xc0015c   ; sym.a166_NEAR.RepeatInit+0x2
       @||   ; CODE XREF from sym.isr_vec_0x8 @ 0xc00008
       @||   ;-- section.PR_TRAPS_FCODE:

Comment thread librz/bin/format/omf/omf166.c Outdated
Comment thread librz/bin/p/bin_c166.c
Comment thread librz/bin/p/bin_omf166.c Outdated
Comment thread librz/bin/p/bin_omf166.c Outdated
Comment thread librz/bin/format/omf/omf166.c Outdated

@wargio wargio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you use AI for this patch?

Comment thread librz/bin/format/omf/omf166.c
@SSharshunov

Copy link
Copy Markdown
Contributor Author

Did you use AI for this patch?

No, I only use AI (gemini) for information retrieval. I write the code myself, and last PR I answered a similar question from Rot127 about why the code sometimes looks a bit messy. Regarding the description, this text was sent as information about the leak, and I've attached it to the description.

@SSharshunov
SSharshunov force-pushed the c166_fix_leaks branch 4 times, most recently from 56c70e3 to 7bdeb9c Compare July 6, 2026 09:57
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.02597% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.94%. Comparing base (de80709) to head (8fdc9c9).

Files with missing lines Patch % Lines
librz/bin/format/omf/omf166.c 79.24% 9 Missing and 2 partials ⚠️
librz/bin/format/omf/omf.c 0.00% 4 Missing and 2 partials ⚠️
librz/bin/p/bin_omf166.c 83.33% 1 Missing and 1 partial ⚠️
librz/arch/isa/c166/c166_disas.c 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
librz/bin/p/bin_c166.c 71.02% <100.00%> (+0.83%) ⬆️
librz/arch/isa/c166/c166_disas.c 90.41% <0.00%> (ø)
librz/bin/p/bin_omf166.c 64.41% <83.33%> (+1.09%) ⬆️
librz/bin/format/omf/omf.c 71.74% <0.00%> (-0.88%) ⬇️
librz/bin/format/omf/omf166.c 64.61% <79.24%> (+0.51%) ⬆️

... and 12 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de80709...8fdc9c9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SSharshunov
SSharshunov force-pushed the c166_fix_leaks branch 4 times, most recently from 93d269a to c5b2832 Compare July 7, 2026 11:58
@wargio
wargio requested a review from notxvilka July 9, 2026 01:52
Comment thread librz/arch/isa/c166/c166_disas.c Outdated
@SSharshunov
SSharshunov force-pushed the c166_fix_leaks branch 2 times, most recently from 5b6dd64 to d4173d9 Compare July 10, 2026 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants