[Rpm-maint] [rpm-software-management/rpm] pgpGet(): check that the returned length is in bounds (#1706)

[Demi Marie Obenour]

@DemiMarie commented on this pull request.



> + * hold `nbytes + *valp` bytes.
+ * @param s		pointer to read from
+ * @param nbytes	length of length field
+ * @param send		pointer past end of buffer
+ * @param[out] *valp	decoded length
+ * @return		0 if buffer can hold `nbytes + *valp` of data,
+ * 			otherwise -1.
+ */
+static int pgpGet(const uint8_t *s, size_t nbytes, const uint8_t *send,
+		  size_t *valp)
+{
+    int rc = -1;
+
+    if (nbytes <= send - s &&
+	nbytes <= sizeof(size_t) &&
+	nbytes <= sizeof(unsigned int)) {

> This looks highly bogus, what does size of size_t have to do with anything PGP?
> Okay, I guess the bogosity comes from pgpGrab() dealing with sizeof(size_t) which looks equally bogus to begin with, going back all the way to [95c55a1](https://github.com/rpm-software-management/rpm/commit/95c55a1af9fa4be56737eefa0e709ce5abe47deb). Looks like well-intended but misguided attempt to size_t type-pedantry, I'm kinda suprised this isn't my fault because I certainly did my share of those in the early days 

I believe this code is correct, but in the interest of accelerating code review, I will leave it out and include it in a separate PR.

> Also the condition aligns on the same indentation level with the following code making it unnecessary hard to read.

How would you prefer that this code be indented?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/1706#discussion_r653811469
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20210617/bca9977a/attachment.html>