Name a raw disassembly's branch targets on every architecture (#439)

CHANGELOG.md

@@ -18,13 +18,20 @@ releases are cut by giving that section a version and a date.
 - Reading a libc that ships without section headers, as an embedded one commonly
   does -- OpenWrt strips them from musl to save flash (#432). Nothing is missing
   from such a file, so the symbols are read through `PT_DYNAMIC` and the code is
-  disassembled as raw bytes; it reports the same gadgets as the same libc with
-  its sections intact.
+  disassembled as raw bytes. i386, amd64 and RISC-V report the same gadgets as
+  the same libc with its sections intact; aarch64 does except where one address
+  carries several symbol names, and arm is not read this way yet.
 - A terminal `posix_spawn` whose symbol carries no version marker, as musl's does
   not, is no longer passed over (#432).
 
 ### Fixed
 
+- A libc with no section headers reported fewer gadgets than the same libc
+  intact on aarch64 and RISC-V (#439). Naming the targets in a raw disassembly
+  covered only the shape amd64 prints, so a target separated by a comma, or
+  followed by the note objdump appends to name a mnemonic's alias, went unnamed
+  and the control-flow walk lost every edge through it -- 84 gadgets across the
+  fixture corpus.
 - amd64 threw away every candidate whose terminal call was not `execve` or
   `posix_spawn` before emulating it -- 10 to 30 `execl` windows per libc, a call
   one_gadget knows how to resolve (#434). They are emulated now. No libc in the

lib/one_gadget/fetchers/base.rb

@@ -103,12 +103,10 @@ module OneGadget
       # stops on ({OneGadget::Emulators::Processor#terminal_call?}). Not
       # {#terminal_call_regexp}, which is looser so it can locate call sites: it
       # also matches the +posix_spawn+ setup helpers, which emulation runs through.
-      # The mnemonic must be a call, so a branch whose target symbol merely looks
-      # similar (+<execlp@@GLIBC_2.4+0x136>+) doesn't end the window.
       # @param [String] line One disassembled line.
       # @return [Boolean]
       def terminal_call_line?(line)
-        return false unless line[/\A\s*[0-9a-f]+:\s*(\S+)/, 1]&.match?(/\A#{call_str}x?(?:\.[wn])?\z/)
+        return false unless call_line?(line)
 
         name = line[/<([^@>]+)/, 1]
         !name.nil? && OneGadget::Emulators::Processor::TERMINAL_CALL_RE.match?(name)
@@ -153,6 +151,26 @@ module OneGadget
 
       private
 
+      # Whether +line+ is a call, whatever it calls. The mnemonic must be the call
+      # itself, so a branch to a symbol whose name merely looks like one is not
+      # counted.
+      # @param [String] line One disassembled line.
+      # @return [Boolean]
+      # @example A call, and a branch into the middle of the same function.
+      #   call_line?('e6570: call   94180 <execve>')          #=> true
+      #   call_line?('a34d0: b      a3210 <execlp+0x1a8>')    #=> false
+      def call_line?(line)
+        line[/\A\s*[0-9a-f]+:\s*(\S+)/, 1]&.match?(/\A#{call_str}x?(?:\.[wn])?\z/) || false
+      end
+
+      # Whether +line+ transfers control, so the address it names is a place in
+      # the file rather than a value it works with.
+      # @param [String] line One disassembled line.
+      # @return [Boolean]
+      def control_transfer?(line)
+        !branch_kind(line).nil? || call_line?(line)
+      end
+
       # Whether +str+ references a libc global, i.e. an address the caller does not
       # choose. The default reads the +$base+-relative form an arch produces once it
       # concretizes a pc-relative operand; one that reaches its globals through a

lib/one_gadget/fetchers/dynamic_symbols.rb

@@ -16,9 +16,21 @@ module OneGadget
     # dynamic segment, and the raw disassembly is rewritten to name them, so
     # everything downstream reads what it always reads.
     module DynamicSymbols
-      # A branch or call whose target objdump could not name, e.g.
-      # +call   0x94180+. Raw disassembly writes every target this way.
-      UNNAMED_TARGET = /\A(.*\s)0x([0-9a-f]+)\z/
+      # Where a line that transfers control says it goes. Raw disassembly writes
+      # every destination as +0x<addr>+, and the rest of the line is whatever the
+      # architecture puts around it: the destination may be separated by a comma,
+      # and followed by the note objdump appends to name the mnemonic's alias.
+      # @example
+      #   'call   0x94180'
+      #   'beqz   a5,0x43060'
+      #   'b.ls   0x47dcc  // b.plast'
+      CONTROL_TARGET = %r{\A(.*[\s,])0x([0-9a-f]+)(?:\s+//.*)?\z}
+
+      # An address any other line names, which it states last and alone -- so a
+      # value the line merely operates on is left as it is written.
+      # @example An architecture may resolve a pc-relative operand from this.
+      #   'lea    rcx,[rip+0x19dabe]        # 0x1eb960'
+      TRAILING_ADDRESS = /\A(.*\s)0x([0-9a-f]+)\z/
 
       private
 
@@ -45,16 +57,21 @@ module OneGadget
       end
 
       # Rewrite a raw-disassembly line into what reading the ELF would have
-      # produced: a target objdump wrote as +0x<addr>+ becomes the bare address,
-      # named when a symbol is there. The engine recognises a terminal call, and
-      # reads a branch target, from exactly that.
+      # produced: an address objdump wrote as +0x<addr>+ becomes the bare address,
+      # named when a symbol is there. The engine recognises a terminal call, reads
+      # a branch target, and matches a safe call by name, from exactly that.
+      #
+      # Where the address sits depends on what the line does, so the two are
+      # matched apart ({CONTROL_TARGET} against {TRAILING_ADDRESS}): asking for a
+      # destination's shape anywhere else would rewrite an operand that only looks
+      # like one.
       # @param [String] line One disassembled line.
       # @param [Hash{Integer => String}] symbols
       # @return [String]
       # @example
       #   symbolize('e6570: call   0x94180', symbols) #=> 'e6570: call   94180 <execve>'
       def symbolize(line, symbols)
-        m = line.match(UNNAMED_TARGET) or return line
+        m = line.match(control_transfer?(line) ? CONTROL_TARGET : TRAILING_ADDRESS) or return line
 
         name = symbols[m[2].to_i(16)]
         named = name ? " <#{name}>" : ''

spec/one_gadget_aarch64_spec.rb

@@ -109,6 +109,10 @@ describe 'one_gadget_aarch64' do
       expect(gadget.constraints).to include('writable: x29-0x20')
       expect(gadget.constraints).not_to include('x2 == 0x1')
     end
+
+    it 'reports the same gadgets for a libc with no section headers' do
+      expect_same_gadgets_when_stripped('aarch64-libc-2.43.so')
+    end
   end
 
   it 'objdump not installed' do

spec/one_gadget_amd64_spec.rb

@@ -1,9 +1,5 @@
 # frozen_string_literal: true
 
-require 'tmpdir'
-
-require 'elftools'
-
 require 'one_gadget'
 require 'one_gadget/error'
 
@@ -166,28 +162,8 @@ describe 'one_gadget_amd64' do
       EOS
     end
 
-    # An embedded libc commonly ships with its section headers removed -- OpenWrt
-    # does it to musl to save flash. Nothing is missing from such a file, but the
-    # usual route to its code and symbols is, so the same libc is stripped here
-    # and required to report the same gadgets.
     it 'reports the same gadgets for a libc with no section headers' do
-      path = data_path('libc-2.31-9fdb74e7b217d06c93172a8243f8547f947ee6d1.so')
-      Dir.mktmpdir do |dir|
-        stripped = File.join(dir, 'stripped.so')
-        File.open(path) do |fd|
-          elf = ELFTools::ELFFile.new(fd)
-          elf.header.e_shoff = 0
-          elf.header.e_shnum = 0
-          elf.header.e_shstrndx = 0
-          elf.save(stripped)
-        end
-        # the state the file is in: objdump disassembles sections, and there are none
-        expect(`objdump -d #{stripped}`).not_to include('call')
-        # Above RAW_LEVEL nothing is trimmed, and every lower level is a function
-        # of that set, so matching here matches at every level.
-        expect(OneGadget.gadgets(file: stripped, force_file: true, level: 100))
-          .to eq OneGadget.gadgets(file: path, force_file: true, level: 100)
-      end
+      expect_same_gadgets_when_stripped('libc-2.31-9fdb74e7b217d06c93172a8243f8547f947ee6d1.so')
     end
   end
 

spec/one_gadget_i386_spec.rb

@@ -82,6 +82,10 @@ describe 'one_gadget_i386' do
     it 'special filename' do
       expect(OneGadget.gadgets(file: data_path('filename$with+special&keys'))).not_to be_empty
     end
+
+    it 'reports the same gadgets for a libc with no section headers' do
+      expect_same_gadgets_when_stripped('libc-2.27-63b3d43ad45e1b0f601848c65b067f9e9b40528b.so')
+    end
   end
 
   describe 'from build id' do

spec/one_gadget_riscv64_spec.rb

@@ -48,5 +48,9 @@ describe 'one_gadget_riscv64' do
       expect(gadget.effect).to eq 'execve("/bin/sh", a1, a2)'
       expect(gadget.constraints).to include('[a1] == NULL || a1 == NULL || a1 is a valid argv')
     end
+
+    it 'reports the same gadgets for a libc with no section headers' do
+      expect_same_gadgets_when_stripped('riscv64-libc-2.39.so')
+    end
   end
 end

spec/spec_helper.rb

@@ -2,6 +2,9 @@
 
 require 'simplecov'
 require 'simplecov_json_formatter'
+require 'tmpdir'
+
+require 'elftools'
 
 SimpleCov.start do
   formatter SimpleCov::Formatter::MultiFormatter.new([
@@ -20,6 +23,7 @@ end
 
 # These requirements must be put after SimpleCov.start,
 # otherwise the coverage report will not include them.
+require 'one_gadget'
 require 'one_gadget/helper'
 require 'one_gadget/logger'
 
@@ -40,6 +44,32 @@ module Helper
   def data_path(file)
     File.join(__dir__, 'data', file)
   end
+
+  # An embedded libc commonly ships with its section headers removed -- OpenWrt
+  # does it to musl to save flash. Nothing is missing from such a file, but the
+  # usual route to its code and symbols is, so +file+ must still report what it
+  # reports intact. Compared at level 100 only: nothing is trimmed above
+  # {OneGadget::Fetchers::RAW_LEVEL}, and every lower level is a function of that
+  # set, so matching there matches at every level.
+  # @param [String] file A fixture name, as {#data_path} takes it.
+  # @return [void]
+  def expect_same_gadgets_when_stripped(file)
+    path = data_path(file)
+    Dir.mktmpdir do |dir|
+      stripped = File.join(dir, 'stripped.so')
+      File.open(path) do |fd|
+        elf = ELFTools::ELFFile.new(fd)
+        elf.header.e_shoff = 0
+        elf.header.e_shnum = 0
+        elf.header.e_shstrndx = 0
+        elf.save(stripped)
+      end
+      # the state the file is in: objdump disassembles sections, and there are none
+      expect(`objdump -d #{stripped}`).not_to match(/^\s*[0-9a-f]+:/)
+      expect(OneGadget.gadgets(file: stripped, force_file: true, level: 100))
+        .to eq OneGadget.gadgets(file: path, force_file: true, level: 100)
+    end
+  end
 end
 
 RSpec.configure do |config|