5 ms·

Note this is for one of the client libraries - the C library, to be specific - not the server (written predominantly in Erlang, a memory-safe language).

by tonyg 7y ago

DyslexicAtheist 7y ago
afaict, the issue is in amqp_handle_input()[1]. The fix was > "add additional input validation to prevent integer overflow when parsing a frame header" Wouldn't this be a bug in the MQ and not in the C library? [1] https://github.com/alanxz/rabbitmq-c/commit/fc85be7123050b91b054e45b91c78d3241a5047a https://github.com/alanxz/rabbitmq-c/commit/fc85be7123050b91... EDIT: sorry I just realized this is an internal rabbit MQ (C) library, I misunderstood the reference to "C library" thinking it meant the systems g/libc
- tonyg 7y ago
  Nope. It's the C library's problem: who is to say which AMQP implementation is at the other end of the socket?
  - DyslexicAtheist 7y ago
    thanks for clarifying, my bad (I misread the meaning of C library)
tonyg 7y ago
(Furthermore, checking the git blame, it turns out this bug is my fault, nearly a decade ago. Decoding a size_t out of the packet and then adding a small constant (7) to it turns out to be able to overflow size_t ... sigh. Programming in C should be against some kind of Geneva Convention.)
- stinos 7y ago
  So it's a typical 'oh a size_t is big enough I'm never going to have numbers that big' bug? Not that I blame you, I've written enough of those..
  - tonyg 7y ago
    More of a "oh yeah, that's right, C doesn't have integers, just machine words, and it doesn't help you avoid errors when you use machine words to simulate integers" bug. Overflow? You get to keep both pieces.
- asveikau 7y ago
  > Programming in C should be against some kind of Geneva Convention Possible and likely in languages other than C. It's just going to hit an out of bounds error when you index the buffer, instead of potentially overwriting an unrelated allocation.
  - tonyg 7y ago
    More that it'd either properly promote from a 64-bit size to a big integer size, or trap on overflow. But either way, yes, a trap is kind of the point. And there are definitely other languages almost equally as toxic as C, yeah.
    - asveikau 7y ago
      I think in this beating a dead horse you fail to see some issues here. If you are going to allocate memory based on the size of a network packet's 32 bit quantity, in any language, that means your connection peer can make you allocate a contiguous 4GB. You already have a bug there. Many languages that readers here consider saviors to big bad C do not make integer overflow fault. It's not as simple a matter as you suggest because there are real tradeoffs. There are also algorithms that rely on wrapping around that are working as intended. Making it fault all the time also means that trap, if not handled, is a bug that may tear down your process, just as that runtime out of bounds exception represents a serious bug too. So another thing to consider. I just tire of hearing that switching your language will make it so you never write any bugs again. A crashing bug in C often would be a crashing bug in another language. The difference is that memory safety gives you better failure modes that don't lead to serious problems like remote code execution. You can make that argument reasonably without pretending the new language will make you infallible and the old language is responsible for all evil in the world.
      - topspin 7y ago
        I don't know this system well enough to say, but 4GB 'frame' size doesn't automatically imply a 4GB buffer must be allocated; the design might be 'cut-through' and only hold a portion of a frame in RAM. For a store and forward service such as a message queue that's actually a likely design. In any case I agree with you; this isn't really about C and isn't a reason to condemn C in favor of something else. This is about trusting user input; the size_t taken from the network PDU/packet/whatever is trusted. That's not valid in any language at any time regardless of how memory safe something else might be.
- farhanhubble 7y ago
  Mathematics in C is modular arithmetic built as an abstraction on top of fixed-width numbers. One has to keep reminding oneself of this fact. This got etched in my brain after I saw a piece of code that calculated the middle index of an array in a roundabout way mid = left+ (right-left) /2 instead of mid = (right +left) / 2 Although even this code can cause overflows, it has served as a reminder to be cautious even with simple addition or increment operations. For this reason the best C code I have seen, an in-memory database with lots of pointer arithmetic, array indexing and bitwise operations, had its own 'safer arithmetic' implemented using macros.
  - fluffything 7y ago
    > One has to keep reminding oneself of this fact. No, one doesn't need to do this, and no matter how hard you try, ths doesn't work in practice since you only need to slip once to introduce a CVE into your program. What we need is for compilers to start calling `abort` by default on any kind of integer overflow. If you have a piece of C code in which you actually need integers to overflow with some particular semantics (e.g. saturating, modulo 2, etc.), you should actually call some standard library API that's lets you express this in a very explicit way. I need to use such APIs every now and then from Rust, but 99% of the time, integer overflow is an error, not something I explicitly intended to introduce undefined behavior or wrap with modulo 2, and as a consequence, if that happens to execute silently, my programs which are not programmed to handle this case will eat your laundry. The suggestion that "you have to keep overflow in mind all the time" is nuts, because most of the time, you have more important things to keep in mind than whether overflowing a singed integer is going to cause UB or whether overflowing an unsigned one is going to wrap around, and manually insert checks everywhere every time you use integers. That's just a completely unreasonable expectation to have about programmers and human beings in general.
    - deleted 7y ago
      [deleted]
    - jimbo1qaz 7y ago
      Doesn't Rust arithmetic wraparound on release? I read Microsoft's Midori language paniced/abandoned on overflow even in release, and the compiler got quite good at optimizing overflow checks away.

Note this is for one of the client libraries - the C library, to be specific - not the server (written predominantly in Erlang, a memory-safe language).

by tonyg 7y ago

Note this is for one of the client libraries - the C library, to be specific - not the server (written predominantly in Erlang, a memory-safe language).

DyslexicAtheist 7y ago

afaict, the issue is in amqp_handle_input()[1]. The fix was > "add additional input validation to prevent integer overflow when parsing a frame header" Wouldn't this be a bug in the MQ and not in the C library? [1] https://github.com/alanxz/rabbitmq-c/commit/fc85be7123050b91b054e45b91c78d3241a5047a https://github.com/alanxz/rabbitmq-c/commit/fc85be7123050b91... EDIT: sorry I just realized this is an internal rabbit MQ (C) library, I misunderstood the reference to "C library" thinking it meant the systems g/libc

tonyg 7y ago
Nope. It's the C library's problem: who is to say which AMQP implementation is at the other end of the socket?
- DyslexicAtheist 7y ago
  thanks for clarifying, my bad (I misread the meaning of C library)

tonyg 7y ago

(Furthermore, checking the git blame, it turns out this bug is my fault, nearly a decade ago. Decoding a size_t out of the packet and then adding a small constant (7) to it turns out to be able to overflow size_t ... sigh. Programming in C should be against some kind of Geneva Convention.)

stinos 7y ago
So it's a typical 'oh a size_t is big enough I'm never going to have numbers that big' bug? Not that I blame you, I've written enough of those..
- tonyg 7y ago
  More of a "oh yeah, that's right, C doesn't have integers, just machine words, and it doesn't help you avoid errors when you use machine words to simulate integers" bug. Overflow? You get to keep both pieces.
asveikau 7y ago
> Programming in C should be against some kind of Geneva Convention Possible and likely in languages other than C. It's just going to hit an out of bounds error when you index the buffer, instead of potentially overwriting an unrelated allocation.
- tonyg 7y ago
  More that it'd either properly promote from a 64-bit size to a big integer size, or trap on overflow. But either way, yes, a trap is kind of the point. And there are definitely other languages almost equally as toxic as C, yeah.
  - asveikau 7y ago
    I think in this beating a dead horse you fail to see some issues here. If you are going to allocate memory based on the size of a network packet's 32 bit quantity, in any language, that means your connection peer can make you allocate a contiguous 4GB. You already have a bug there. Many languages that readers here consider saviors to big bad C do not make integer overflow fault. It's not as simple a matter as you suggest because there are real tradeoffs. There are also algorithms that rely on wrapping around that are working as intended. Making it fault all the time also means that trap, if not handled, is a bug that may tear down your process, just as that runtime out of bounds exception represents a serious bug too. So another thing to consider. I just tire of hearing that switching your language will make it so you never write any bugs again. A crashing bug in C often would be a crashing bug in another language. The difference is that memory safety gives you better failure modes that don't lead to serious problems like remote code execution. You can make that argument reasonably without pretending the new language will make you infallible and the old language is responsible for all evil in the world.
    - topspin 7y ago
      I don't know this system well enough to say, but 4GB 'frame' size doesn't automatically imply a 4GB buffer must be allocated; the design might be 'cut-through' and only hold a portion of a frame in RAM. For a store and forward service such as a message queue that's actually a likely design. In any case I agree with you; this isn't really about C and isn't a reason to condemn C in favor of something else. This is about trusting user input; the size_t taken from the network PDU/packet/whatever is trusted. That's not valid in any language at any time regardless of how memory safe something else might be.
farhanhubble 7y ago
Mathematics in C is modular arithmetic built as an abstraction on top of fixed-width numbers. One has to keep reminding oneself of this fact. This got etched in my brain after I saw a piece of code that calculated the middle index of an array in a roundabout way mid = left+ (right-left) /2 instead of mid = (right +left) / 2 Although even this code can cause overflows, it has served as a reminder to be cautious even with simple addition or increment operations. For this reason the best C code I have seen, an in-memory database with lots of pointer arithmetic, array indexing and bitwise operations, had its own 'safer arithmetic' implemented using macros.
- fluffything 7y ago
  > One has to keep reminding oneself of this fact. No, one doesn't need to do this, and no matter how hard you try, ths doesn't work in practice since you only need to slip once to introduce a CVE into your program. What we need is for compilers to start calling `abort` by default on any kind of integer overflow. If you have a piece of C code in which you actually need integers to overflow with some particular semantics (e.g. saturating, modulo 2, etc.), you should actually call some standard library API that's lets you express this in a very explicit way. I need to use such APIs every now and then from Rust, but 99% of the time, integer overflow is an error, not something I explicitly intended to introduce undefined behavior or wrap with modulo 2, and as a consequence, if that happens to execute silently, my programs which are not programmed to handle this case will eat your laundry. The suggestion that "you have to keep overflow in mind all the time" is nuts, because most of the time, you have more important things to keep in mind than whether overflowing a singed integer is going to cause UB or whether overflowing an unsigned one is going to wrap around, and manually insert checks everywhere every time you use integers. That's just a completely unreasonable expectation to have about programmers and human beings in general.
  - deleted 7y ago
    [deleted]
  - jimbo1qaz 7y ago
    Doesn't Rust arithmetic wraparound on release? I read Microsoft's Midori language paniced/abandoned on overflow even in release, and the compiler got quite good at optimizing overflow checks away.