search

protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
64.54k stars 15.37k forks source link

NULL pointer checks are needed after allocating memory(encode_decode.c) #6548

Closed leo960 closed 4 years ago

leo960 commented 4 years ago

When I go through the source code, I found that there's no failure check after using malloc() and which may lead to crash. So I suggest that the following cases should add some checks for NULL:

  1. https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L135
  2. https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L157
  3. https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L170
  4. https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L193
  5. https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L654
TeBoring commented 4 years ago

Would you mind sending us a fix?

wangkirin commented 4 years ago

Besides @leo960 listed above, I think there are also some other code should be fix https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L76 https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L248 https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L359 https://github.com/protocolbuffers/protobuf/blob/63e4a3ecc956cbab6714b25e8b868765ea7e6fe5/php/ext/google/protobuf/encode_decode.c#L768

And Adding assertion after malloc may make the code more strong.

wangkirin commented 4 years ago

Hi, @TeBoring It seems @leo960 doesn't have enough time to fix this issue. I am happy to give this issue a shot~~

TeBoring commented 4 years ago

Thanks for your help!

On Mon, Aug 26, 2019 at 19:21 Wang Qilin notifications@github.com wrote:

Hi, @TeBoring https://github.com/TeBoring It seems @leo960 https://github.com/leo960 doesn't have enough time to fix this issue. I am happy to give this issue a shot~~

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/protocolbuffers/protobuf/issues/6548?email_source=notifications&email_token=ABHUPZJNNFMZDD3ECRFDCHTQGSFSXA5CNFSM4IOR3CA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5GIBSY#issuecomment-525107403, or mute the thread https://github.com/notifications/unsubscribe-auth/ABHUPZM3X6O52O63QO4X5I3QGSFSXANCNFSM4IOR3CAQ .