1. 04 Nov, 2021 3 commits
  2. 28 Oct, 2021 2 commits
    • Victor Toso's avatar
      usbredirhost: restrain setting buffered_output_size_cb · f298f497
      Victor Toso authored
      
      
      Application should not be able to set this callback if data queue is
      on the library's side.
      Signed-off-by: Victor Toso's avatarVictor Toso <victortoso@redhat.com>
      f298f497
    • Victor Toso's avatar
      usbredirect: let usbredirparser owns the write buffer · a01820bc
      Victor Toso authored
      The flag usbredirhost_fl_write_cb_owns_buffer is used to set
      usbredirparser's usbredirparser_fl_write_cb_owns_buffer. When this
      flag is used, then usbredirect owns the data from the write callback
      (usbredir_write_cb) and it should free() that data after using it.
      
      At the moment, we are leaking this data:
      
       > 72,440 bytes in 27 blocks are possibly lost in loss record 1,870 of 1,872
       >    at 0x484086F: malloc (vg_replace_malloc.c:380)
       >    by 0x4DCA07E: usbredirparser_queue (usbredirparser.c:1176)
       >    by 0x4853FF7: usbredirhost_send_stream_data (usbredirhost.c:1092)
       >    by 0x48573DF: usbredirhost_iso_packet_complete (usbredirhost.c:1544)
       >    by 0x487DE74: usbi_handle_transfer_completion (io.c:1692)
       >    by 0x487E2AE: UnknownInlinedFun (linux_usbfs.c:2658)
       >    by 0x487E2AE: reap_for_handle (linux_usbfs.c:2693)
       >    by 0x487EC6D: op_handle_events (linux_usbfs.c:2757)
       >    by 0x487FCE1: handle_events (io.c:2254)
       >    by 0x4880E36: libusb_handle...
      a01820bc
  3. 16 Oct, 2021 1 commit
    • Fabrice Fontaine's avatar
      meson.build: make C++ optional · 55fc307d
      Fabrice Fontaine authored
      Remove cpp from meson project statement to make C++ optional and avoid
      the following build failure when the toolchain does not provide a C++
      compiler:
      
      ../output-1/build/usbredir-0.11.0/meson.build:1:0: ERROR: Unknown compiler(s): [['/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++']]
      The following exception(s) were encountered:
      Running "/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++ --version" gave "[Errno 2] No such file or directory: '/home/buildroot/autobuild/instance-3/output-1/host/bin/arm-linux-g++'"
      
      Indeed C++ is only required for fuzzing which is already handled by
      meson through add_languages('cpp', required: true)
      
      Fixes:
       - http://autobuild.buildroot.org/results/eca1d8a2b73a769354ab1d24c7996be30f152138
      
      Signed-off-by: Fabrice Fontaine's avatarFabrice Fontaine <fontaine.fabrice@gmail.com>
      55fc307d
  4. 02 Oct, 2021 1 commit
  5. 29 Sep, 2021 2 commits
  6. 28 Sep, 2021 1 commit
  7. 24 Sep, 2021 1 commit
  8. 28 Aug, 2021 2 commits
    • hansmi's avatar
      Avoid memory leak from ill-formatted serialization data · 186c4c79
      hansmi authored
      At commit 060d7914
      
       the following `fuzzing/usbredirparserfuzz` input triggers
      a memory leak:
      
      ```
      $ base64 -d <<'EOF' | gunzip -c > testcase
      H4sIAFDwJ2ECA2NgZIADBQWF/zD2HQgfAv4DgcJ/BTzgP0iRAlwRivL/ePX9
      R1VAyBaSjcShB8nPTBA9/xn+g5UAANvH+dkSAQAA
      ```
      
      The data type header segment is empty while there's (supposed) payload data in
      there. The internal buffers must be filled in the order of header, type header
      and then data, an invariant important for `usbredirparser_do_read` and violated
      by this input.
      
      With this change the input data is read the same way, but if the invariant would
      be violated the data read is just ignored.
      
      The parser check at the beginning of `usbredirparser_unserialize` is also
      improved and `write_buf_count` is no longer set explicitly.
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      186c4c79
    • hansmi's avatar
      usbredirparser: Use consistent indentation · c3c67aba
      hansmi authored
      
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      c3c67aba
  9. 23 Aug, 2021 2 commits
    • hansmi's avatar
      Update write buffer count during deserialization · 060d7914
      hansmi authored
      At commit 8490a7ac
      
       the following `fuzzing/usbredirparserfuzz` input causes
      an integer underflow on write_buf_count as it's not updated during
      deserialization:
      
      ```
      $ base64 -d <<'EOF' | gunzip -c > testcase6250181968920576
      H4sIAMChImECAzNkwA0YgZgTxPiPBGCSAkgK07b9YgPRLDA+EBvNYoPLuzDg
      A3Cj3/xHBTc5IPJgFQz/vwNJznSGBIYQIBtJPxN29n842xbhaohtaXA7GX4z
      fHsPFNVmYHiI4S8jBiKAADbfAEMOaEkCMEDArv6fBpX4gzMM/6OoxxHOWMMO
      YjYiIFleosvhsxcD/IYofY/dW0AD/xPy1nWc3kpDTUsEnYPD+UQEPzHuxB78
      qWDwn0pgP8k6UvGDq0Alu7Foe0+0BYygEDBg+LwtAZhdAUAa2Kf/AwAA
      EOF
      ```
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      060d7914
    • hansmi's avatar
      Skip empty write buffers when unserializing parser · e37d86c2
      hansmi authored
      At commit 8490a7ac
      
       the following `fuzzing/usbredirparserfuzz` input causes
      the instantiation of empty write buffers:
      
      $ base64 -d <<'EOF' > testcase6474540506021888
      QAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAG
      AAAAAN7/AAAACAA=
      EOF
      
      Empty write buffers trigger all kinds of issues, in part because they cause
      calls to write(2) with a zero length.
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      e37d86c2
  10. 22 Aug, 2021 1 commit
  11. 21 Aug, 2021 1 commit
  12. 13 Aug, 2021 5 commits
  13. 12 Aug, 2021 1 commit
  14. 09 Aug, 2021 1 commit
  15. 08 Aug, 2021 1 commit
    • hansmi's avatar
      Avoid use-after-free in serialization · 03c519ff
      hansmi authored
      Serializing parsers with large amounts of buffered write data (e.g. in case of
      a slow or blocked write destination) would cause "serialize_data" to reallocate
      the state buffer whose default size is 64kB (USBREDIRPARSER_SERIALIZE_BUF_SIZE).
      The pointer to the position for the write buffer count would then point to
      a location outside the buffer where the number of write buffers would be written
      as a 32-bit value.
      
      As of QEMU 5.2.0 the serializer is invoked for migrations. Serializations for
      migrations may happen regularily such as when using the COLO feature[1].
      Serialization happens under QEMU's I/O lock. The guest can't control the state
      while the serialization is happening. The value written is the number of
      outstanding buffers which would be suceptible to timing and host system system
      load. The guest would have to continously groom the write buffers. A useful
      value needs to be allocated in the exact position freed during the buffer size
      increase, but before the buffer count is written. The author doesn't consider it
      realistic to exploit this use-after-free reliably.
      
      [1] https://wiki.qemu.org/Features/COLO
      
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      03c519ff
  16. 08 Jul, 2021 6 commits
  17. 20 Jun, 2021 1 commit
    • hansmi's avatar
      Add fuzzer for filters · 70978842
      hansmi authored
      
      
      Filters are exposed to the peer and parser bugs could be dangerous. None were
      found during the implementation of this fuzzer.
      
      A minor change is made to the filter code to reject separators which aren't
      exactly one character long (as documented as a requirement).
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      70978842
  18. 19 Jun, 2021 2 commits
    • hansmi's avatar
      Verify that rule separators are not empty · 68d143f5
      hansmi authored
      
      
      The `usbredirfilter_rules_to_string` function always dereferences its separator
      arguments, thus requiring at least one character.
      
      Parsing a rule string in `usbredirfilter_string_to_rules` can only work
      correctly when there's at least one separator character for both rules and
      tokens.
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      68d143f5
    • hansmi's avatar
      Use single stateless function for filter tests · b0ee1209
      hansmi authored
      
      
      Recent commits have added unittests for the filter parsing code. The test code
      contained two similar, but different implementations of table-driven tests. With
      the way the code was written it was rather hard to identify which exact case
      failed.
      
      With this change every test case runs under its own "testpath", making it
      recognizable from the output (e.g. "ok 28 /filter/rules/bad/17"). In addition
      the evaluated filter is emitted in escaped form as a debug message.
      
      The previous implementation of the test for acceptable filter rules was
      stateful, i.e. changing the token or rule separator would have an effect on
      later test cases. In order to have a predictable test situation it's better
      to always fully specify a full test case.
      Signed-off-by: hansmi's avatarMichael Hanselmann <public@hansmi.ch>
      b0ee1209
  19. 18 Jun, 2021 3 commits
  20. 09 Jun, 2021 1 commit
  21. 07 Jun, 2021 2 commits