devongovett / exif-reader

A small EXIF image metadata reader
MIT License
152 stars 22 forks source link

Don't try to use a non-integer as an offset into the buffer #14

Closed papandreou closed 5 years ago

papandreou commented 5 years ago

Caught by #6:

  1) fuzz tests should parse or reject a randomly mutated EXIF data chunk based on the tetons fixture:

Found an error after 938 iterations, 1 additional error found.
counterexample:

  Generated input: Buffer.from([0x45, 0x78, 0x69, 0x66, 0x00, 0x00, 0x4D, 0x4D, 0x00, 0x2A, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0B /* 8142 more */ ])
  with: fuzz({
    value: Buffer.from([0x45, 0x78, 0x69, 0x66, 0x00, 0x00, 0x4D, 0x4D, 0x00, 0x2A, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0B /* 8142 more */ ]),
    mutator: integer({ min: 1, max: 10 }).map(function (numMutations) {
      var mutatedBuffer = Buffer.from(buffer); // Make a copy
      for (var i = 0 ; i < numMutations ; i += 1) {
        var octetNumber = g.integer({min: 0, max: buffer.length}).first();
        mutatedBuffer[octetNumber] = g.integer({min: 0, max: 255}).first();
      }
      return mutatedBuffer;
    })
  })

  expected Buffer.from([0x45, 0x78, 0x69, 0x66, 0x00, 0x00, 0x4D, 0x4D, 0x00, 0x2A, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0B /* 8142 more */ ]) to either parse or throw documented error
    Threw unexpected error: TypeError [ERR_INVALID_ARG_TYPE]: The "offset" argument must be of type number. Received type string
        at validateNumber (internal/validators.js:130:11)
        at Buffer.readUInt16BE (internal/buffer.js:211:3)
        at readUInt16 (/home/andreas/work/exif-reader/index.js:212:19)
        at readTags (/home/andreas/work/exif-reader/index.js:65:20)
        at module.exports (/home/andreas/work/exif-reader/index.js:44:21)
        at Object.handler (/home/andreas/work/exif-reader/test/index.js:152:14)
[...]

I can provide a regression test with the specific malformed EXIF chunks that provoke this, but if the intention is to merge #6, it's also well covered by that.

papandreou commented 5 years ago

@lovell, heh, this was actually the thing you ran into when you first tried https://github.com/devongovett/exif-reader/pull/6#issuecomment-535133122 -- it's just a genuine bug :)

lovell commented 5 years ago

Brilliant, it all makes sense now, thank you.