Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket28755 v2: K=V parsing with fuzzer #577

Closed
wants to merge 9 commits into from

Conversation

Labels
None yet
Projects
None yet
4 participants
@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Dec 10, 2018

No description provided.

nmathewson added 2 commits Dec 10, 2018
encoding and decoding.

There are bunches of places where we don't want to invest in a full
fuzzer, but we would like to make sure that some string operation
can handle all its possible inputs.  This fuzzer uses the first byte
of its input to decide what to do with the rest of the input.  Right
now, all the possibilities are decoding a string, and seeing whether
it is decodeable.  If it is, we try to re-encode it and do the whole
thing again, to make sure we get the same result.

This turned up a lot of bugs in the key-value parser, and I think it
will help in other cases too.

Closes ticket 28808.
@coveralls
Copy link

@coveralls coveralls commented Dec 10, 2018

Pull Request Test Coverage Report for Build 3275

  • 78 of 79 (98.73%) changed or added relevant lines in 1 file are covered.
  • 1335 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.07%) to 60.783%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/encoding/kvline.c 78 79 98.73%
Files with Coverage Reduction New Missed Lines %
src/feature/nodelist/networkstatus.c 8 52.87%
src/core/or/versions.c 11 94.42%
src/feature/control/control.c 1316 22.22%
Totals Coverage Status
Change from base Build 3214: 0.07%
Covered Lines: 43501
Relevant Lines: 71568

💛 - Coveralls

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

#include <stdio.h>//XXXX
Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

rm that :)

v = line->value;
}

//printf(" <%s!%s!%s>::\n", k,eq,v);//XXXX
Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

Leftovers


char *result = smartlist_join_strings(elements, " ", 0, NULL);

//printf("::%s::\n", result);//XXXX
Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

Leftovers

* allocated list of pairs on success, or NULL on failure.
*
* If KV_QUOTED is set in <b>flags</b>, then quoted values are
* allowed. Otherwise, such values are not allowed.
Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

The single quote is not parsed so KV_QUOTED implies double quotes. Is the term "Quoted" usually implies only double quotes?

Copy link
Contributor Author

@nmathewson nmathewson Dec 12, 2018

That's right; I've added a clarifying comment


lines = kvline_parse("A\"B=C CDE=\"F\" \"GHI\" ", KV_QUOTED|KV_OMIT_KEYS);
tt_assert(! lines);

Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

So I've tried this and the first one works but the second one fails where I indicated. I'm guessing "AB=" should be allowed meaning an empty value?

  lines = kvline_parse("AB=", KV_QUOTED);
  tt_assert(lines);
  tt_str_op(lines->key, OP_EQ, "AB");
  tt_str_op(lines->value, OP_EQ, "");

  lines = kvline_parse("AB=", 0);
  tt_assert(lines); <--- FAILURE IS HERE
  tt_str_op(lines->key, OP_EQ, "AB");
  tt_str_op(lines->value, OP_EQ, "");

#ifndef TOR_KVLINE_H
#define TOR_KVLINE_H

struct smartlist_t;
Copy link
Contributor

@dgoulet-tor dgoulet-tor Dec 11, 2018

Don't think this is needed in the header.

nmathewson added 5 commits Dec 12, 2018
Allow empty values when a key is present, but not otherwise.

With test from dgoulet.
Remove debugging printfs and stdio include
@torproject-pusher torproject-pusher deleted the branch torproject:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment