[Libguestfs] [PATCH] lib: update inspect_list_applications to return all installed RPMs (RHBZ#859885)

[John Eckersberg]

"Richard W.M. Jones" <rjones at redhat.com> writes:
>> +
>> +static char *
>> +get_rpm_header_tag (guestfs_h *g, const void *header_start, size_t header_len, uint32_t tag)
>> +{
>> +  uint32_t num_fields, offset;
>> +  const void *cursor = header_start + 8, *store;
>> +
>> +  /* This function parses the RPM header structure to pull out various
>> +   * tag strings (version, release, arch, etc.).  For more detail on the
>> +   * header format, see:
>> +   * http://www.rpm.org/max-rpm/s1-rpm-file-format-rpm-file-format.html#S2-RPM-FILE-FORMAT-HEADER
>> +   */
>> +
>> +  num_fields = be32toh (*(uint32_t *) header_start);
>> +  store = header_start + 8 + (16 * num_fields);
>> +
>> +  while (cursor < store && cursor < header_start + header_len) {
>> +    if (be32toh (*(uint32_t *) cursor) == tag){
>
> ^ Space before '{' character.
>
>> +      offset = be32toh(*(uint32_t *) (cursor + 8));
>> +      return safe_strdup(g, store + offset);
>> +    }
>> +    cursor += 16;
>> +  }
>
> I'm curious if this code will work if header_len is unusually small.
> I think it would cause the library to read past the end of the
> allocated buffer, possibly crashing or doing other Bad Stuff.  Note
> that the header_len field is under control of the guest, so this could
> be a security problem.

Do you mean unusually large?  The while loop is bounded by:

cursor < header_start + header_len

An unusually small header_len will just cause it to bail early and
return NULL, which is not a problem.