Unidata / UDUNITS-2

API and utility for arithmetic manipulation of units of physical quantities
http://www.unidata.ucar.edu/software/udunits
Other
62 stars 36 forks source link

galileanNew assert triggered in fuzzing #66

Closed schwehr closed 6 years ago

schwehr commented 6 years ago

In doing a little initial fuzzing of udunits I hit this assert of scale within seconds. Asserts should be for things that should never happen. My fix below probably isn't the best, but it at least lets me continue on with fuzzing.

static ut_unit*
galileanNew(
    double      scale,
    const ut_unit*  unit,
    double      offset)
{
    ut_unit*    newUnit = NULL; /* failure */

    /* assert(scale != 0); */
    if (scale == 0) return NULL;

The fuzzer (which isn't particularly good for coverage):

#include <stddef.h>
#include <stdint.h>
#include <string>
#include <vector>

extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
  const string data2(reinterpret_cast<const char *>(data), size);

  ut_set_error_message_handler(ut_ignore);

  ut_system *unit_system = ut_new_system();
  auto unit_system_cleaner =
      gtl::MakeCleanup([unit_system] { ut_free_system(unit_system); });

  ut_unit *unit = ut_parse(unit_system, data2.c_str(), UT_ASCII);
  auto unit_cleaner =
      gtl::MakeCleanup([unit] { ut_free(unit); });

  constexpr size_t kBufSize = 1000;
  std::vector<char> buf(kBufSize);
  CHECK_GE(static_cast<int>(kBufSize), ut_format(unit, &buf[0], kBufSize, 0));

  return 0;
}

Some of the inputs to ut_parse that trigger the assert include 360-191, 229991-091

semmerson commented 6 years ago

The next release will have the assert()s replaced with a conditional and an error-return.

schwehr commented 6 years ago

Can you please link the fix so that I can match your fix to my code base and drop my hack?

semmerson commented 6 years ago

I'm not sure what you're asking by "Link the fix"?

If you're asking for a new release, then I'm not ready for that yet.

schwehr commented 6 years ago

Can you not push that commit? Or if not, quote the code in the bug? That will save me a merge conflict

semmerson commented 6 years ago

@schwehr, I don't understand. That code was committed about 6 hours ago. See https://github.com/Unidata/UDUNITS-2/blob/master/lib/unitcore.c

schwehr commented 6 years ago

I assumed you had not pushed the code as I had assumed that you had put #66 in the commit message, but you hadn't. I've now added a comment to the commit to make it obvious.

semmerson commented 6 years ago

@schwehr Thanks. I committed before I knew about the special commit words.