News for hackers. Stuff that matters.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by audidude on Saturday June 25, 2016 @07:52PM (#11978666)
// SECURITY IMPROVEMENTS REQUIRED. VULNERABLE TO INTEGER AND BUFFER OVERFLOWS.

well at least it's pretty clear up front. No malloc result checks, invalid format in printfs, probably more.

(I don't mean to be disparaging, please don't take it that way OP. We clearly need more/better tooling in this area in C).

(Edited for clarity)

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by userbinator on Saturday June 25, 2016 @11:17PM (#11979246)
No malloc result checks

The last time I worked with the DNS protocol, I seem to remember that everything had a well-defined (and small) limit on its size, and it wasn't really necessary to use dynamic allocation at all. Strings are length-prefixed, not null-terminated, and the longest length fields are only 16 bits.

A look at the RFC shows the strict limits on size:

https://www.ietf.org/rfc/rfc1035.txt

In comparison to text protocols like HTTP, I see far less opportunities to make errors like buffer overflows with DNS. It's a simple and relatively easy to parse binary protocol.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by legulere on Sunday June 26, 2016 @07:17AM (#11980316)
Just because strings are length prefixed, doesn't mean that they don't need to be dynamically allocated. The reason they can be put on the stack is that the prefix is just one byte and thus the maximum length 255 bytes for the strings.

Prefixed strings can even be a source of buffer overflows, if you don't check if the length is smaller than the actual length of the rest of the data. In fact this was actually the source of the heartbleed bug.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by tptacek on Saturday June 25, 2016 @08:49PM (#11978868)
Malloc result checks are generally a bad idea; they're error prone. Better just to run with the allocator configured to abort on failure.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by audidude on Saturday June 25, 2016 @11:40PM (#11979288)
Generally agree, (and that's what we usually do in GLib). However, for a general purpose DNS user-space implementation I think it would be good to have something that can be used in both scenarios.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by tptacek on Sunday June 26, 2016 @09:04PM (#11983639)
That's a coherent argument, but that argument is also, I think, probably responsible for virtually all of the exploitable offset-from-NULL vulnerabilities shipped in C code.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by jjnoakes on Saturday June 25, 2016 @11:30PM (#11979267)
Unless, of course, one can recover from malloc failing.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by pcwalton on Sunday June 26, 2016 @01:01PM (#11981551)
Even if you can in theory, it's still a bad idea to try to do that. Statistically speaking, you will get it wrong.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by jjnoakes on Sunday June 26, 2016 @05:06PM (#11982722)
It may be a bad idea for an average developer to try it, sure. Plenty of advanced things fall in to that category though.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by pcwalton on Sunday June 26, 2016 @06:41PM (#11983141)
It is a bad idea for any developer to try it. Statistically speaking, it is the best programmers who create the most widespread security flaws.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by briansmith on Sunday June 26, 2016 @07:51PM (#11983424)
Which statistics are you referring to?
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by pcwalton on Sunday June 26, 2016 @09:15PM (#11983682)
Well, there can't be any objective quantification of "best", so it's impossible to prove this without being subjective. But the most widespread vulnerabilities tend to be in software that a lot of people use. Software that a lot of people use is usually written by good programmers, because good programmers (for typical definitions of "good") are the most productive ones.

The people who write the Linux kernel, for example, are by and large good programmers, because kernel programming is difficult and becoming a prolific contributor even more so. They've been responsible for the majority of the security bugs in the Linux kernel. That doesn't make them bad programmers.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by jjnoakes on Sunday June 26, 2016 @11:11PM (#11983998)
Correlation vs causation.

Good programmers work on popular software because most people work on popular software.

Most security bugs are found in popular software because most people care about (use, audit, exploit) popular software.

Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

I'd argue that the reverse is most likely true. Good programmers know their limits, the limits of their tools, and have the experience to recognize problematic patterns.

Saying no one should try to recover from allocation failure is like saying no one should attempt surgery because only surgeons kill people occasionally during operations.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by kibwen on Monday June 27, 2016 @12:01AM (#11984150)

  > Nothing here backs the assertion that good programmers 
  > are more likely to create more bugs per line of code 
  > written.
This is moving the goalposts. Here's your original statement:

"It may be a bad idea for an average developer to try it"

And here's pcwalton's rebuttal:

"It is a bad idea for any developer to try it"

The relative frequency of bugs between average developers and advanced developers isn't the issue. Very good developers create bugs that lead to security flaws all the time. That the number of bugs they create is less than the number of bugs that would be created by less experienced developers does not matter in this context, because we have empirical evidence that both these numbers are very far from zero.

  > Good programmers know their limits, the limits of 
  > their tools, and have the experience to recognize 
  > problematic patterns
Sharing such experience is precisely what tptacek and pcwalton are doing here.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by jjnoakes on Monday June 27, 2016 @08:01AM (#11985588)
I didn't move the goalposts very far at all. If you quoted the rest of pcwalton's comment you would have had a more appropriate context in which to understand my reply.

Here we have a claim that encompasses 100% of a population, which is refuted by someone in that population who doesn't agree with the claim (and whose direct experience contradicts it).

And it isn't just my experience which contradicts it. Many of my colleagues would dismiss such an assertion as well.

Now if his claim was about "many" or even "most" programmers then sure, perhaps there is nothing to refute. But then his claim would have added almost nothing to what he replied to.

Saying all? That's either a dramatic device which I don't think adds to a healthy discussion, or a genuine belied that I think should be refuted since I'm in a position to provide counter evidence.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by pcwalton on Monday June 27, 2016 @12:06PM (#11987087)
> Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

That isn't what I said. I said that good programmers create more bugs on the whole.

> Saying no one should try to recover from allocation failure is like saying no one should attempt surgery because only surgeons kill people occasionally during operations.

We need surgery so that people don't die. You probably don't need to recover from malloc failing.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by jjnoakes on Monday June 27, 2016 @03:08PM (#11988618)
> > Nothing here backs the assertion that good programmers are more likely to create more bugs per line of code written.

> I said that good programmers create more bugs on the whole.

Well if good programmers write more code, then sure, that follows quite nicely. If you don't normalize for lines of code written, though, then I'm not sure I follow what the point of that assertion is.

If a good programmer writes 10x the code but 1.5x the bugs, I don't think it's fair or meaningful for this discussion to claim that the good programmer created more bugs on the whole.

> We need surgery so that people don't die. You probably don't need to recover from malloc failing.

Software definitely runs in places where lives can be affected and lost.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by technion on Sunday June 26, 2016 @04:37AM (#11979906)

    We clearly need more/better tooling in this area in C).
I think the tooling we have nowadays is better than we give it credit for. Between clang's analyser, ASAN, Infer and tis-interpreter, I often find when someone points out a vulnerability, I can get one of these tools to make an issue of it.

Note I'm not suggesting this is enough to make C safe.

Edit: A quick demonstration, based on 30 seconds with valgrind. If I run with ./bin/massdns --help, it makes a malloc() on line 595, then exits on 605 without free()ing. And yes I know this isn't really an issue in practice, but it demonstrates that the tools can see issues.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by blechschmidt on Saturday June 25, 2016 @07:57PM (#11978687)
Malloc results are checked by the safe_malloc function in security.h. Could you point out a line that is prone to integer overflows?
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by audidude on Saturday June 25, 2016 @08:00PM (#11978697)
stream_buffer_init() calls malloc directly, so it's not protected by safe_malloc(). (Unless there is some funky re-define stuff I didn't see).

%x takes unsigned int, not a char. (Although I guess you could just rely on promotion here if you can rely on the unsignedness of char).

> Could you point out a line that is prone to integer overflows?

Never said that it was an overflow (just C&P that comment from the file). Just said its invalid use. Which I'm totally willing to agree on the compiler mostly getting correct behavior. Just not really something I'd want to see landing in something that we all need a really good, secure implementation of.

Best of luck though, I hope you iterate on this to the point we can have a really good DNS implementation in C, in user-space, with optional async processing.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by userbinator on Saturday June 25, 2016 @11:20PM (#11979252)
Although I guess you could just rely on promotion here if you can rely on the unsignedness of char

If compilers did not do promotion, they would be very obviously not compliant with the C standard and unable to compile correctly most existing code, so I'm willing to bet that is something you can rely on. It's defined by the standard and widely used.

Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by audidude on Saturday June 25, 2016 @11:37PM (#11979284)
Sure, but in this case it was signed to unsigned promotion.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by blechschmidt on Saturday June 25, 2016 @11:43PM (#11979294)
Why should that matter in this case? The worst case would have been the output of a wrong value and it was some commented out debug output which I have used in a very specific case.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by audidude on Sunday June 26, 2016 @12:01AM (#11979321)
Totally, just wrong value. This thread is already making a bigger deal of it than I expected it to.
Re:Show HN: MassDNS – A high-performance DNS stub resolver in C by blechschmidt on Saturday June 25, 2016 @08:11PM (#11978739)
Thank you for pointing this out. This file actually contained some unused code that was not supposed to make it into the repository so I just removed it. The "%x" format specifiers were located in debug comments that have been commented out. They have also been removed now.