Checksum validation, more tests

lib/rex/proto/acpp/message.rb

@@ -89,57 +89,23 @@ module ACPP
     #
     # @return [String] the string representation of this Message
     def to_s
-      'acpp' + [
-        1, # unknown1
-        message_checksum,
-        payload_checksum,
-        @payload.size,
-        0, 0, # unknown2
-        @type,
-        @status,
-        0, 0, 0 # unknown3
-      ].pack('NNNNN2NNN3') +
-      Rex::Encoding::Xor::Generic.encode([@password].pack('a32').slice(0, 32), XOR_KEY).first +
-      ([0] * 12).pack('N12') + # unknown4
-      payload
+      return with_checksum(Zlib::adler32(with_checksum(0)))
     end
 
     # Compares this Message and another Message for equality
     #
     # @param other [Message] the Message to compare
-    # @return [Boolean] true iff the two messages have equal String representations, false otherwise
+    # @return [Boolean] true iff the two messages are equal, false otherwise
     def ==(other)
-      to_s == other.to_s
-    end
-
-    private
-
-    # compute the 32-bit checksum of the payload
-    def payload_checksum
-      Zlib::adler32(@payload)
-    end
-
-    # compute the 32-bit checksum of the entire message and payload
-    def message_checksum
-      Zlib::adler32(
-        'acpp' + [
-        1, # unknown1
-        0, # message checksum is set to 0 during checksum calculation
-        payload_checksum,
-        @payload.size,
-        0, 0, # unknown2
-        @type,
-        @status,
-        0, 0, 0 # unknown3
-        ].pack('NNNNN2NNN3') +
-        Rex::Encoding::Xor::Generic.encode([@password].pack('a32').slice(0, 32), XOR_KEY).first +
-        ([0] * 12).pack('N12') + # unknown4
-        payload
-      )
+      other.type == @type &&
+      other.status == @status &&
+      other.password == @password &&
+      other.payload == @payload
     end
 
     def self.decode(data, validate_checksum = true)
-      fail "Incorrect ACPP message size #{data.size}" unless data.size == 128
+      data = data.dup
+      fail "Incorrect ACPP message size #{data.size} -- must be 128" unless data.size == 128
       fail 'Unexpected header' unless 'acpp' == data.slice!(0, 4)
       unknown1 = data.slice!(0, 4)
       read_message_checksum = data.slice!(0, 4).unpack('N').first
@@ -151,13 +117,46 @@ module ACPP
       unknown3 = data.slice!(0, 12)
       password = Rex::Encoding::Xor::Generic.encode(data.slice!(0, 32), XOR_KEY).first.strip
       unknown4 = data.slice!(0, 48)
+      payload = data
       m = self.new
       m.type = type
       m.password = password
       m.status = status
-      m.payload = data
+      m.payload = payload
+
+      # we can now validate the checksums if desired
+      if validate_checksum
+        actual_message_checksum = Zlib::adler32(m.with_checksum(0))
+        if actual_message_checksum != read_message_checksum
+          fail "Invalid message checksum (expected #{read_message_checksum}, got #{actual_message_checksum})"
+        end
+        # I'm not sure this can ever happen -- if the payload checksum is wrong, then the
+        # message checksum will also be wrong.  So, either I misunderstand the protocol
+        # or having two checksums is useless
+        actual_payload_checksum = Zlib::adler32(payload)
+        if actual_payload_checksum != read_payload_checksum
+          fail "Invalid payload checksum (expected #{read_payload_checksum}, got #{actual_payload_checksum})"
+        end
+      end
+      m
       m
     end
+
+    def with_checksum(message_checksum)
+        'acpp' + [
+        1, # unknown1
+        message_checksum,
+        Zlib::adler32(payload),
+        @payload.size,
+        0, 0, # unknown2
+        @type,
+        @status,
+        0, 0, 0 # unknown3
+        ].pack('NNNNN2NNN3') +
+        Rex::Encoding::Xor::Generic.encode([@password].pack('a32').slice(0, 32), XOR_KEY).first +
+        ([0] * 12).pack('N12') + # unknown4
+        payload
+    end
   end
 end
 end

spec/lib/rex/proto/acpp/message_spec.rb

@@ -9,17 +9,31 @@ describe Rex::Proto::ACPP::Message do
     described_class.new
   end
 
-  # public.bin has the contents of a message type 20, password public message
-  # with a 612-byte retrieve payload that happens to be in a different packet
+  # retrieve_public.bin has the contents of a message type 20 (retrieve
+  # settings) message with a password of public.  There is no payload.
   let(:retrieve_public_bin) do
     IO.read(File.join(File.dirname(__FILE__), 'retrieve_public.bin'))
   end
 
   let(:retrieve_public_message) do
-    m = described_class.new
-    m.password = 'public'
-    m.type = 20
-    m
+    message.password = 'public'
+    message.type = 20
+    message
+  end
+
+  describe '#==' do
+    it 'considers two different objects composed of equal parts equal' do
+      message2 = described_class.new
+      message2.password = 'public'
+      message2.type = 20
+      expect(message2).to eq(retrieve_public_message)
+    end
+    it 'considers two different objects composed of different parts unequal' do
+      message3 = described_class.new
+      message3.type = 1
+      message3.password = 'private'
+      expect(message3).not_to eq(retrieve_public_message)
+    end
   end
 
   describe '#to_s' do
@@ -29,8 +43,27 @@ describe Rex::Proto::ACPP::Message do
   end
 
   describe '#decode' do
-    it 'decodes properly' do
+    it 'fails to decode if the message is the wrong size' do
+      small = Rex::Text.rand_text_alpha(100)
+      large = Rex::Text.rand_text_alpha(200)
+      expect { described_class.decode(small) }.to raise_error(/size #{small.size}/i)
+      expect { described_class.decode(large) }.to raise_error(/size #{large.size}/i)
+    end
+    it 'fails to decode if the required header is incorrect' do
+      retrieve_public_bin[0,4] = 'blah'
+      expect { described_class.decode(retrieve_public_bin) }.to raise_error(/header/i)
+    end
+    it 'decodes properly when the required checksum is correct' do
       expect(retrieve_public_message).to eq(described_class.decode(retrieve_public_bin))
+      expect(retrieve_public_message).to eq(described_class.decode(retrieve_public_bin, true))
+    end
+    it 'decodes properly when the non-required checksum is correct' do
+      expect(retrieve_public_message).to eq(described_class.decode(retrieve_public_bin, false))
+    end
+    it 'fails to decode the required message checksum is incorrect' do
+      retrieve_public_bin[7,4] = "\x01\x02\x03\x04"
+      expect { described_class.decode(retrieve_public_bin) }.to raise_error(/message checksum/i)
+      expect { described_class.decode(retrieve_public_bin, true) }.to raise_error(/message checksum/i)
     end
   end
 end