[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 17 Jun 2024 19:38:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Reviewed-on: http://gerrit.cloudera.org:8080/21501 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 89 insertions(+), 17 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10725/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 17 Jun 2024 14:28:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 17 Jun 2024 14:28:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Mon, 17 Jun 2024 13:30:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16348/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:44:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h@642 PS2, Line 642: unsigned > It seems safer to not change this as it is a public const. Done -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:19:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 89 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21501/3 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h File be/src/udf/udf.h: http://gerrit.cloudera.org:8080/#/c/21501/2/be/src/udf/udf.h@642 PS2, Line 642: uint64_t It seems safer to not change this as it is a public const. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Fri, 14 Jun 2024 14:06:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16330/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Thu, 13 Jun 2024 13:16:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 90 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21501/2 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Peter Rozsa has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/21501/1/be/src/udf/udf-test.cc File be/src/udf/udf-test.cc: http://gerrit.cloudera.org:8080/#/c/21501/1/be/src/udf/udf-test.cc@340 PS1, Line 340: dummpy typo: dummy http://gerrit.cloudera.org:8080/#/c/21501/1/be/src/udf/udf-test.cc@344 PS1, Line 344: } nit: could be one line -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Thu, 13 Jun 2024 11:04:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21501 ) Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/16312/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Noemi Pap-Takacs Gerrit-Reviewer: Peter Rozsa Gerrit-Comment-Date: Tue, 11 Jun 2024 09:51:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom()
Daniel Becker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21501 Change subject: IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() .. IMPALA-13150: Possible buffer overflow in StringVal::CopyFrom() In StringVal::CopyFrom(), we take the 'len' parameter as a size_t, which is usually a 64-bit unsigned integer. We pass it to the constructor of StringVal, which takes it as an int, which is usually a 32-bit signed integer. The constructor then allocates memory for the length using the int value, but afterwards in CopyFrom(), we copy the buffer with the size_t length. If size_t is indeed 64 bits and int is 32 bits, and the value is truncated, we may copy more bytes that what we have allocated for the destination. Note that in the constructor of StringVal it is checked whether the length is greater than 1GB, but if the value is truncated because of the type conversion, the check doesn't necessarily catch it as the truncated value may be small. This change fixes the problem by doing the length check with 64 bit integers in StringVal::CopyFrom(). Testing: - added unit tests for StringVal::CopyFrom() in udf-test.cc. Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 --- M be/src/udf/udf-test.cc M be/src/udf/udf.cc M be/src/udf/udf.h 3 files changed, 91 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/21501/1 -- To view, visit http://gerrit.cloudera.org:8080/21501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6a1d03d65ec4339a0f33e69ff29abdd8cc3e3067 Gerrit-Change-Number: 21501 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Becker
