4 ms·

Malloc results are checked by the safe_malloc function in security.h. Could you point out a line that is prone to integer overflows?

audidude 10y ago
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.
- blechschmidt 10y ago
  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.
- userbinator 10y ago
  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.
  - audidude 10y ago
    Sure, but in this case it was signed to unsigned promotion.
    - blechschmidt 10y ago
      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.
      - audidude 10y ago
        Totally, just wrong value. This thread is already making a bigger deal of it than I expected it to.

audidude 10y ago

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.

blechschmidt 10y ago
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.
userbinator 10y ago
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.
- audidude 10y ago
  Sure, but in this case it was signed to unsigned promotion.
  - blechschmidt 10y ago
    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.
    - audidude 10y ago
      Totally, just wrong value. This thread is already making a bigger deal of it than I expected it to.