Re: [PATCH 3/6] Make sure fsck_commit_buffer() does not run out of the buffer
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 28, 2014, 20:59 UTC
- Message-ID
- <xmqqr4008h4p.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <alpine.DEB.1.00.1408281646450.990@s15462909.onlinehome-server.info>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 12 quoted lines
> So far, we assumed that the buffer is NUL terminated, but this is not > a safe assumption, now that we opened the fsck_object() API to pass a > buffer directly. > > So let's make sure that there is at least an empty line in the buffer. > That way, our checks would fail if the empty line was encountered > prematurely, and consequently we can get away with the current string > comparisons even with non-NUL-terminated buffers are passed to > fsck_object(). > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > ---
Heh, I probably should have read this one before commenting on 2/6.
It makes me feel somewhat uneasy to insist that there must be a blank line in the commit object, even though from the very first implementation of "commit-tree" I think we have always had a blank there at the end of the header, even when you feed nothing as the message to it.
I think the new check is OK, but the message should be s/empty line/end of header/ or something. It is not like we require an empty line in the log message proper.
Show 44 quoted lines
> fsck.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/fsck.c b/fsck.c
> index dd77628..db6aaa4 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -237,6 +237,26 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
> return retval;
> }
>
> +static int must_have_empty_line(const void *data, unsigned long size,
> + struct object *obj, fsck_error error_func)
> +{
> + const char *buffer = (const char *)data;
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + switch (buffer[i]) {
> + case '\0':
> + return error_func(obj, FSCK_ERROR,
> + "invalid message: NUL at offset %d", i);
> + case '\n':
> + if (i + 1 < size && buffer[i + 1] == '\n')
> + return 0;
> + }
> + }
> +
> + return error_func(obj, FSCK_ERROR, "invalid buffer: missing empty line");
> +}
> +
> static int fsck_ident(const char **ident, struct object *obj, fsck_error error_func)
> {
> char *end;
> @@ -284,6 +304,9 @@ static int fsck_commit_buffer(struct commit *commit, const char *buffer,
> unsigned parent_count, parent_line_count = 0;
> int err;
>
> + if (must_have_empty_line(buffer, size, &commit->object, error_func))
> + return -1;
> +
> if (!skip_prefix(buffer, "tree ", &buffer))
> return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");
> if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')