On 03/02/10 18:34, Richard W.M. Jones wrote:
Subject: [PATCH 09/12] hivex: Begin implementation of writing to
hives.
This implements hivex_node_set_values which is used to
delete the (key, value) pairs at a node and optionally
replace them with a new set.
This also implements hivex_commit which is used to commit
changes to hives back to disk.
---
hivex/hivex.c | 384 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hivex/hivex.h | 12 ++
hivex/hivex.pod | 125 ++++++++++++++++++
3 files changed, 521 insertions(+), 0 deletions(-)
This looks generally ok. However, it is *extremely* dense. I've
indicated a few places where I feel there are insufficient comments, or
magic numbers making the code less clear than it could be.
+/* Allocate an hbin (page), extending the malloc'd space if
necessary,
+ * and updating the hive handle fields (but NOT the hive disk header
+ * -- the hive disk header is updated when we commit). This function
+ * also extends the bitmap if necessary.
+ *
+ * 'allocation_hint' is the size of the block allocation we would like
+ * to make. Normally registry blocks are very small (avg 50 bytes)
+ * and are contained in standard-sized pages (4KB), but the registry
+ * can support blocks which are larger than a standard page, in which
+ * case it creates a page of 8KB, 12KB etc.
+ *
+ * Returns:
+ * > 0 : offset of first usable byte of new page (after page header)
+ * 0 : error (errno set)
+ */
+static size_t
+allocate_page (hive_h *h, size_t allocation_hint)
+{
Every usage of nr_4k_pages in this function multiplies it by 4096. It
would be more readable to choose a different name which reflects that
it's simply a rounding.
+ /* In almost all cases this will be 1. */
+ size_t nr_4k_pages =
+ 1 + (allocation_hint + sizeof (struct ntreg_hbin_page) - 1) / 4096;
+ assert (nr_4k_pages >= 1);
I think this assert is impossible to trigger.
+ /* 'extend' is the number of bytes to extend the file by.
Note that
+ * hives found in the wild often contain slack between 'endpages'
+ * and the actual end of the file, so we don't always need to make
+ * the file larger.
+ */
+ ssize_t extend = h->endpages + nr_4k_pages * 4096 - h->size;
+
+ if (h->msglvl >= 2) {
+ fprintf (stderr, "allocate_page: current endpages = 0x%zx, current size =
0x%zx\n",
+ h->endpages, h->size);
+ fprintf (stderr, "allocate_page: extending file by %zd bytes (<= 0 if no
extension)\n",
+ extend);
+ }
+
+ if (extend > 0) {
+ size_t oldsize = h->size;
+ size_t newsize = h->size + extend;
In general, are we worrying about overflow?
+ char *newaddr = realloc (h->addr, newsize);
+ if (newaddr == NULL)
+ return 0;
+
+ size_t oldbitmapsize = 1 + oldsize / 32;
+ size_t newbitmapsize = 1 + newsize / 32;
+ char *newbitmap = realloc (h->bitmap, newbitmapsize);
+ if (newbitmap == NULL) {
+ free (newaddr);
+ return 0;
+ }
+
+ h->addr = newaddr;
+ h->size = newsize;
+ h->bitmap = newbitmap;
+
+ memset (h->addr + oldsize, 0, newsize - oldsize);
+ memset (h->bitmap + oldbitmapsize, 0, newbitmapsize - oldbitmapsize);
+ }
+
+ size_t offset = h->endpages;
+ h->endpages += nr_4k_pages * 4096;
+
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_page: new endpages = 0x%zx, new size =
0x%zx\n",
+ h->endpages, h->size);
+
+ /* Write the hbin header. */
+ struct ntreg_hbin_page *page =
+ (struct ntreg_hbin_page *) (h->addr + offset);
+ page->magic[0] = 'h';
+ page->magic[1] = 'b';
+ page->magic[2] = 'i';
+ page->magic[3] = 'n';
+ page->offset_first = htole32 (offset - 0x1000);
What's 0x1000? Can you comment this?
+ page->page_size = htole32 (nr_4k_pages * 4096);
+ memset (page->unknown, 0, sizeof (page->unknown));
+
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_page: new page at 0x%zx\n", offset);
+
+ /* Offset of first usable byte after the header. */
+ return offset + sizeof (struct ntreg_hbin_page);
+}
+
+/* Allocate a single block, first allocating an hbin (page) at the end
+ * of the current file if necessary. NB. To keep the implementation
+ * simple and more likely to be correct, we do not reuse existing free
+ * blocks.
+ *
+ * seg_len is the size of the block (this INCLUDES the block header).
+ * The header of the block is initialized to -seg_len (negative to
+ * indicate used). id[2] is the block ID (type), eg. "nk" for nk-
+ * record. The block bitmap is updated to show this block as valid.
+ * The rest of the contents of the block will be zero.
+ *
+ * Returns:
+ * > 0 : offset of new block
+ * 0 : error (errno set)
+ */
+static size_t
+allocate_block (hive_h *h, size_t seg_len, const char id[2])
+{
+ if (!h->writable) {
+ errno = EROFS;
+ return 0;
+ }
+
+ if (seg_len < 4) {
Can you replace this '4' with a sizeof(ntreg_hbin_block.seg_len)?
+ /* The caller probably forgot to include the header. Note that
+ * value lists have no ID field, so seg_len == 4 would be possible
+ * for them, albeit unusual.
+ */
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_block: refusing too small allocation (%zu),
returning ERANGE\n",
+ seg_len);
+ errno = ERANGE;
+ return 0;
+ }
+
+ /* Refuse really large allocations. */
+ if (seg_len > 1000000) {
Can you replace 1000000 with a #define?
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_block: refusing large allocation (%zu), returning
ERANGE\n",
+ seg_len);
+ errno = ERANGE;
+ return 0;
+ }
+
+ /* Round up allocation to multiple of 8 bytes. All blocks must be
+ * on an 8 byte boundary.
+ */
+ seg_len = (seg_len + 7) & ~7;
+
+ /* Allocate a new page if necessary. */
+ if (h->endblocks == 0 || h->endblocks + seg_len > h->endpages) {
+ size_t newendblocks = allocate_page (h, seg_len);
+ if (newendblocks == 0)
+ return 0;
+ h->endblocks = newendblocks;
+ }
+
+ size_t offset = h->endblocks;
+
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_block: new block at 0x%zx, size %zu\n",
+ offset, seg_len);
+
+ struct ntreg_hbin_block *blockhdr =
+ (struct ntreg_hbin_block *) (h->addr + offset);
+
+ blockhdr->seg_len = htole32 (- (int32_t) seg_len);
+ if (id[0] && id[1] && seg_len >= 6) {
Can you replace 6 with sizeof(ntreg_hbin_block)?
+ blockhdr->id[0] = id[0];
+ blockhdr->id[1] = id[1];
+ }
+
+ h->endblocks += seg_len;
+
+ /* If there is space after the last block in the last page, then we
+ * have to put a dummy free block header here to mark the rest of
+ * the page as free.
+ */
+ ssize_t rem = h->endpages - h->endblocks;
+ if (rem > 0) {
+ if (h->msglvl >= 2)
+ fprintf (stderr, "allocate_block: marking remainder of page free starting at
0x%zx, size %zd\n",
+ h->endblocks, rem);
+
+ assert (rem >= 4);
This assertion doesn't look safe. Why must this be the case? In the
event that rem < 4, would you allocate a new page for the dummy free block?
+ blockhdr = (struct ntreg_hbin_block *) (h->addr +
h->endblocks);
+ blockhdr->seg_len = htole32 ((int32_t) rem);
Will blockhdr->id ever be read here? Is 0x00 valid?
+ }
+
+ return offset;
+}
+
+
+/* Delete all existing values at this node. */
+static int
+delete_values (hive_h *h, hive_node_h node)
+{
+ assert (h->writable);
+
+ hive_value_h *values;
+ size_t *blocks;
+ if (get_values (h, node, &values, &blocks) == -1)
+ return -1;
+
+ size_t i;
+ for (i = 0; blocks[i] != 0; ++i)
+ mark_block_unused (h, blocks[i]);
+
+ free (blocks);
+
+ for (i = 0; values[i] != 0; ++i) {
+ struct ntreg_vk_record *vk =
+ (struct ntreg_vk_record *) (h->addr + values[i]);
+
+ size_t len;
+ len = le32toh (vk->data_len);
+ if (len == 0x80000000) /* special case */
+ len = 4;
+ len &= 0x7fffffff;
Comment, please. What's going on with this record length?
+
+ if (len > 4) { /* non-inline, so remove data block */
+ size_t data_offset = le32toh (vk->data_offset);
+ data_offset += 0x1000;
+ mark_block_unused (h, data_offset);
More magic numbers :) Why is this necessary?
+ }
+
+ /* remove vk record */
+ mark_block_unused (h, values[i]);
+ }
+
+ free (values);
+
+ struct ntreg_nk_record *nk = (struct ntreg_nk_record *) (h->addr + node);
+ nk->nr_values = htole32 (0);
+ nk->vallist = htole32 (0xffffffff);
+
+ return 0;
+}
+
+int
+hivex_commit (hive_h *h, const char *filename, int flags)
+{
+ if (flags != 0) {
+ errno = EINVAL;
+ return -1;
+ }
What are you intending to use the flags for?
+ if (!h->writable) {
+ errno = EROFS;
+ return -1;
+ }
+
+ filename = filename ? : h->filename;
+ int fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666);
+ if (fd == -1)
+ return -1;
+
+ /* Update the header fields. */
+ uint32_t sequence = le32toh (h->hdr->sequence1);
+ sequence++;
+ h->hdr->sequence1 = htole32 (sequence);
+ h->hdr->sequence2 = htole32 (sequence);
+ /* XXX Ought to update h->hdr->last_modified. */
+ h->hdr->blocks = htole32 (h->endpages - 0x1000);
+
+ /* Recompute header checksum. */
+ uint32_t sum = header_checksum (h);
+ h->hdr->csum = htole32 (sum);
+
+ if (h->msglvl >= 2)
+ fprintf (stderr, "hivex_commit: new header checksum: 0x%x\n", sum);
+
+ if (full_write (fd, h->addr, h->size) != h->size) {
+ int err = errno;
+ close (fd);
+ errno = err;
+ return -1;
+ }
+
+ if (close (fd) == -1)
+ return -1;
+
+ return 0;
+}
+
+int
+hivex_node_set_values (hive_h *h, hive_node_h node,
+ size_t nr_values, const hive_set_value *values,
+ int flags)
+{
+ if (!h->writable) {
+ errno = EROFS;
+ return -1;
+ }
+
+ if (!IS_VALID_BLOCK (h, node) || !BLOCK_ID_EQ (h, node, "nk")) {
+ errno = EINVAL;
+ return -1;
This would be an internal inconsistency error, surely. If so, you
probably want an assert here instead.
+ }
+
+ /* Delete all existing values. */
+ if (delete_values (h, node) == -1)
+ return -1;
+
+ if (nr_values == 0)
+ return 0;
+
+ /* Allocate value list node. Value lists have no id field. */
+ static const char nul_id[2] = { 0, 0 };
+ size_t seg_len =
+ sizeof (struct ntreg_value_list) + (nr_values - 1) * sizeof (uint32_t);
Why are you subtracting 1 from nr_values above?
+ size_t vallist_offs = allocate_block (h, seg_len, nul_id);
+ if (vallist_offs == 0)
+ return -1;
+
+ struct ntreg_nk_record *nk = (struct ntreg_nk_record *) (h->addr + node);
+ nk->nr_values = htole32 (nr_values);
+ nk->vallist = htole32 (vallist_offs - 0x1000);
Whatever the purpose of adding or subtracting 0x1000 to offsets is, it
could probably do with a descriptively-named macro, which also does
htole32 or le32toh as appropriate.
+ struct ntreg_value_list *vallist =
+ (struct ntreg_value_list *) (h->addr + vallist_offs);
+
+ size_t i;
+ for (i = 0; i < nr_values; ++i) {
+ /* Allocate vk record to store this (key, value) pair. */
+ static const char vk_id[2] = { 'v', 'k' };
+ seg_len = sizeof (struct ntreg_vk_record) + strlen (values[i].key);
+ size_t vk_offs = allocate_block (h, seg_len, vk_id);
+ if (vk_offs == 0)
+ return -1;
+
+ vallist->offset[i] = htole32 (vk_offs - 0x1000);
+
+ struct ntreg_vk_record *vk = (struct ntreg_vk_record *) (h->addr + vk_offs);
+ size_t name_len = strlen (values[i].key);
If you did this further up you could re-use the result of strlen.
+ vk->name_len = htole16 (name_len);
+ strcpy (vk->name, values[i].key);
+ vk->data_type = htole32 (values[i].t);
+ vk->data_len = htole16 (values[i].len);
+ vk->flags = name_len == 0 ? 0 : 1;
+
+ if (values[i].len <= 4) /* Store data inline. */
sizeof.
+ memcpy (&vk->data_offset, values[i].value,
values[i].len);
+ else {
+ size_t offs = allocate_block (h, values[i].len + 4, nul_id);
sizeof.
+ if (offs == 0)
+ return -1;
+ memcpy (h->addr + offs + 4, values[i].value, values[i].len);
sizeof, or the address of a struct member.
+ vk->data_offset = htole32 (offs - 0x1000);
+ }
+
+ if (name_len * 2 > le32toh (nk->max_vk_name_len))
+ nk->max_vk_name_len = htole32 (name_len * 2);
Why is name_len multiplied by 2? Comment, please.
+ if (values[i].len > le32toh (nk->max_vk_data_len))
+ nk->max_vk_data_len = htole32 (values[i].len);
+ }
+
+ return 0;
+}
diff --git a/hivex/hivex.h b/hivex/hivex.h
index 56718b4..6a3cb3a 100644
--- a/hivex/hivex.h
+++ b/hivex/hivex.h
@@ -110,6 +110,18 @@ struct hivex_visitor {
extern int hivex_visit (hive_h *h, const struct hivex_visitor *visitor, size_t len, void
*opaque, int flags);
extern int hivex_visit_node (hive_h *h, hive_node_h node, const struct hivex_visitor
*visitor, size_t len, void *opaque, int flags);
+extern int hivex_commit (hive_h *h, const char *filename, int flags);
+
+struct hive_set_value {
+ char *key;
+ hive_type t;
+ size_t len;
+ char *value;
+};
+typedef struct hive_set_value hive_set_value;
+
+extern int hivex_node_set_values (hive_h *h, hive_node_h node, size_t nr_values, const
hive_set_value *values, int flags);
+
#ifdef __cplusplus
}
#endif
diff --git a/hivex/hivex.pod b/hivex/hivex.pod
index 5a58144..5df75aa 100644
--- a/hivex/hivex.pod
+++ b/hivex/hivex.pod
@@ -326,6 +326,127 @@ starts at C<node>.
=back
+=head2 WRITING TO HIVE FILES
+
+The hivex library supports making limited modifications to hive files.
+We have tried to implement this very conservatively in order to reduce
+the chance of corrupting your registry. However you should be careful
+and take back-ups, since Microsoft has never documented the hive
+format, and so it is possible there are nuances in the
+reverse-engineered format that we do not understand.
+
+To be able to modify a hive, you must pass the C<HIVEX_OPEN_WRITE>
+flag to C<hivex_open>, otherwise any write operation will return with
+errno C<EROFS>.
+
+The write operations shown below do not modify the on-disk file
+immediately. You must call C<hivex_commit> in order to write the
+changes to disk. If you call C<hivex_close> without committing then
+any writes are discarded.
+
+Hive files internally consist of a "memory dump" of binary blocks
+(like the C heap), and some of these blocks can be unused. The hivex
+library never reuses these unused blocks. Instead, to ensure
+robustness in the face of the partially understood on-disk format,
+hivex only allocates new blocks after the end of the file, and makes
+minimal modifications to existing structures in the file to point to
+these new blocks. This makes hivex slightly less disk-efficient than
+it could be, but disk is cheap, and registry modifications tend to be
+very small.
+
+When deleting nodes, it is possible that this library may leave
+unreachable live blocks in the hive. This is because certain parts of
+the hive disk format such as security (sk) records and big data (db)
+records and classname fields are not well understood (and not
+documented at all) and we play it safe by not attempting to modify
+them. Apart from wasting a little bit of disk space, it is not
+thought that unreachable blocks are a problem.
+
+=over 4
+
+=item int hivex_commit (hive_h *h, const char *filename, int flags);
+
+Commit (write) any changes which have been made.
+
+C<filename> is the new file to write. If C<filename == NULL> then we
+overwrite the original file (ie. the file name that was passed to
+C<hivex_open>). C<flags> is not used, always pass 0.
+
+Returns 0 on success. On error this returns -1 and sets errno.
+
+Note this does not close the hive handle. You can perform further
+operations on the hive after committing, including making more
+modifications. If you no longer wish to use the hive, call
+C<hivex_close> after this.
+
+=item hive_set_value
+
+The typedef C<hive_set_value> is used in conjunction with the
+C<hivex_node_set_values> call described below.
+
+ struct hive_set_value {
+ char *key; /* key - a UTF-8 encoded ASCIIZ string */
+ hive_type t; /* type of value field */
+ size_t len; /* length of value field in bytes */
+ char *value; /* value field */
+ };
+ typedef struct hive_set_value hive_set_value;
+
+To set the default value for a node, you have to pass C<key = "">.
+
+Note that the C<value> field is just treated as a list of bytes, and
+is stored directly in the hive. The caller has to ensure correct
+encoding and endianness, for example converting dwords to little
+endian.
+
+The correct type and encoding for values depends on the node and key
+in the registry, the version of Windows, and sometimes even changes
+between versions of Windows for the same key. We don't document it
+here. Often it's not documented at all.
+
+=item int hivex_node_set_values (hive_h *h, hive_node_h node, size_t nr_values, const
hive_set_value *values, int flags);
+
+This call can be used to set all the (key, value) pairs stored in C<node>.
+
+C<node> is the node to modify. C<values> is an array of (key, value)
+pairs. There should be C<nr_values> elements in this array. C<flags>
+is not used, always pass 0.
+
+Any existing values stored at the node are discarded, and their
+C<hive_value_h> handles become invalid. Thus you can remove all
+values stored at C<node> by passing C<nr_values = 0>.
+
+Returns 0 on success. On error this returns -1 and sets errno.
+
+Note that this library does not offer a way to modify just a single
+key at a node. We don't implement a way to do this efficiently.
+
+=back
+
+=head3 WRITE OPERATIONS WHICH ARE NOT SUPPORTED
+
+=over 4
+
+=item *
+
+Changing the root node.
+
+=item *
+
+Creating a new hive file from scratch. This is impossible at present
+because not all fields in the header are understood.
+
+=item *
+
+Modifying or deleting single values at a node.
+
+=item *
+
+Modifying security key (sk) records or classnames. These are not
+well understood.
+
+=back
+
=head1 THE STRUCTURE OF THE WINDOWS REGISTRY
Note: To understand the relationship between hives and the common
@@ -452,6 +573,10 @@ Registry contains cycles.
Field in the registry out of range.
+=item EROFS
+
+Tried to write to a registry which is not opened for writing.
+
=back
=head1 ENVIRONMENT VARIABLES
-- 1.6.5.2
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490