#69 Minor - limit ReadHeaderField() loop

closed
nobody
None
5
2014-08-25
2010-09-01
Shawn Collenburg
No

The loop:

while\(true\)
\{
    if\(ReadHeaderField\(br\) == false\)
        break;
\}

presumes Kdb4HeaderFieldID.EndOfHeader will be found.

Recommendation -- limit the loop:

for\(int iFields = 100; iFields > 0; i--\)
\{
    if\(ReadHeaderField\(br\) == false\)
        break;
\}

* Please don't limit this to strictly 10. In order to implement the multiple user keys plugin ( https://sourceforge.net/tracker/?func=detail&aid=3042335&group_id=95013&atid=609911 ) I will likely need to stuff extra MasterSeed / TransformSeed pairs into the header [ As long as the plugin causes the last pair to be consistent you could still open the database without the particular pluging, e.g. café mode ]

Discussion

    • priority: 5 --> 3
     
  • Dominik Reichl
    Dominik Reichl
    2010-09-01

    I don't see why this should be limited. Such limitations almost always cause problems in the future. Also, if it would be limited and the maximum count is reached, what does KeePass do then? The header hasn't been read completely, i.e. the only thing KeePass can do is to fail with an error message. Not reasonable in my opinion.

    By the way, these header fields aren't plugin-extensible. On the one hand there will most likely be conflicts if you're using the header, and on the other hand KeePass will show a warning dialog when it encounters an unknown header field. If you need to store additional data, put it into a separate file or use the structures intended for plugins (like PwDatabase.CustomData, but this of course is only readable *after* decrypting the file, so my suggestion would be to use a separate file to store additional unencrypted data).

    Best regards
    Dominik

     
  • Dominik Reichl
    Dominik Reichl
    2010-09-01

    • priority: 3 --> 5
    • status: open --> closed