darcs

Issue 2576 PatchInfo parsing is broken / needs validation

Title PatchInfo parsing is broken / needs validation
Priority Status resolved
Milestone Resolved in 2.15.0
Superseder Nosy List bfrk
Assigned To
Topics

Created on 2018-03-17.17:03:11 by bfrk, last changed 2020-06-20.10:44:00 by bfrk.

Messages
msg19977 (view) Author: bfrk Date: 2018-03-17.17:03:09
I have been looking into PatchInfo parsing. It seems the parsing can be
easily broken leading to corrupt repos.

For instance, I just discovered that the patch name must not end with
']' or else unrecording the patch will hang in an endless loop. There
are no checks and you can create such a patch with darcs. (Also tested
with 2.10 so this is not a recent regression.)

Even worse things happen if you pass a non-ASCII date e.g. via --pipe.

This is quite horrible. The constraints required by the current
PatchInfo parser are nowhere documented and nowhere checked before
creating PatchInfos. I have a patch for some of this badness since a few
days but that doesn't cover the example above yet.
msg19978 (view) Author: bfrk Date: 2018-03-17.17:22:35
The example I gave is bogus, I tested it with a repo that was already
broken. Sorry. The point about validation stands, though, there are
certain restrictions to be observed, mainly for author and date.
msg19982 (view) Author: bfrk Date: 2018-03-18.20:59:20
Here is a summary of what the PatchInfo items may contain in order for
readPatchInfo to be able to parse back what showPatchinfo writes to disk.

date: any ASCII(!) character except '\n' or ']'
name,log: anything but '\n'
author: anything but '*'

The ASCII restriction for date is due to the use of BC.pack/unpack. The
other restrictions are necessary because the parser uses these
characters as delimiters of the fields in question. For the log the
restriction applies to each line of the log, of course.

Violating any of these invariants when constructing a PatchInfo can lead
to unparsable PatchInfos written to disk which is absolutely fatal for
users.
msg19984 (view) Author: bfrk Date: 2018-03-18.21:21:14
See Patch1662
msg22059 (view) Author: bfrk Date: 2020-06-20.10:43:57
resolved by

patch 403698ffdc7b41b9840ccad632915f82a848b644
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Mar 12 22:42:53 CET 2018
  * add validation for PatchInfo items, add unit test for 
parse/unparse
History
Date User Action Args
2018-03-17 17:03:11bfrkcreate
2018-03-17 17:22:36bfrksetmessages: + msg19978
title: PatchInfo parsing is broken / need validation -> PatchInfo parsing is broken / needs validation
2018-03-18 20:59:22bfrksetmessages: + msg19982
2018-03-18 21:21:15bfrksetmessages: + msg19984
2020-06-20 10:44:00bfrksetstatus: unknown -> resolved
messages: + msg22059
resolvedin: 2.15.0