Title: plistlib crashes too easily on bad files
Type: behavior Stage: resolved
Components: Macintosh Versions: Python 3.1, Python 3.2, Python 3.3
Status: closed Resolution: fixed
Dependencies: 775321 Superseder:
Assigned To: ned.deily Nosy List: eric.araujo, ezio.melotti, georg.brandl, jackjansen, jvr, mher.movsisyan, ned.deily, python-dev, ronaldoussoren
Priority: normal Keywords: easy, patch

Created on 2004-07-04 21:30 by jackjansen, last changed 2011-05-28 10:19 by ned.deily. This issue is now closed.

File name Uploaded Description Edit
plist_validation.diff mher.movsisyan, 2010-12-10 10:30 patch for validation review
plist_validation_v2.diff mher.movsisyan, 2011-01-04 15:01 patch review
Messages (8)
msg60527 - (view) Author: Jack Jansen (jackjansen) * (Python committer) Date: 2004-07-04 21:30
Plistlib doesn't do much error checking, and it can crash on bad 
input. Moreover, it doesn't provide much help if it does crash (no 
linenumbers, etc).

The problem I ran into was a dangling <key>foo</key>. After this 
key the dict ended, but the next entry in the surrounding 
datastructure, an array, picked up the key from self.currentKey 
and crashed in addObject().

I was about to fix this when I noticed that there's lots of problems 
with <key> handling, duplicates or missing ones aren't detected 
either and can cause crashes too. It may be better to put a general 
try/except in parse() and print a line number or something in case 
of a failure.
msg123725 - (view) Author: Mher Movsisyan (mher.movsisyan) Date: 2010-12-10 10:30
The attached patch fixes crashes on bad input. The patch implements validation for dict and array elements as well as some resource cleanup. The tests are included as well.
msg123758 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2010-12-10 23:25
One review comment: the patch adds a new exception class that is used for the errors that are now additionally detected.  Elsewhere plistlib uses non-specific exception classes like ValueError.  If starting from scratch, it might be better to consistently use a specific exception class but that would create incompatibilities if changed now.  I don't see a compelling need to add one now just for these errors.  (But, if kept, it should be added to the docs.)  Otherwise, looks good to me.

Thanks for taking this on!
msg125333 - (view) Author: Mher Movsisyan (mher.movsisyan) Date: 2011-01-04 15:01
I've replaced plistlib.InvalidPlistError with ValueError
msg125341 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-04 16:54
msg126089 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2011-01-12 12:39
See also reopened dependency #775321.
msg137115 - (view) Author: Roundup Robot (python-dev) Date: 2011-05-28 10:10
New changeset a2688e252204 by Ned Deily in branch '3.1':
Issue #985064: Make plistlib more resilient to faulty input plists.

New changeset f555d959a5d7 by Ned Deily in branch '3.2':
Issue #985064: Make plistlib more resilient to faulty input plists.

New changeset d0bc18a50bd1 by Ned Deily in branch 'default':
Issue #985064: Make plistlib more resilient to faulty input plists.
msg137116 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2011-05-28 10:19
Thank you for the patch and tests!  Applied in 3.1 (for 3.1.4), 3.2 (for 3.2.1), and 3.3. (The 2.x version of plistlib differs somewhat from the 3.x version so the patch would need some rework and testing for 2.7; that is probably not worth the effort at this point.)
Date User Action Args
2011-05-28 10:19:39ned.deilysetstatus: open -> closed
versions: + Python 3.3, - Python 2.7
messages: + msg137116

assignee: ned.deily
resolution: fixed
stage: patch review -> resolved
2011-05-28 10:10:40python-devsetnosy: + python-dev
messages: + msg137115
2011-04-11 03:54:39ezio.melottisetnosy: + ezio.melotti
2011-04-11 01:57:48terry.reedysetassignee: jvr -> (no value)
2011-01-12 12:39:18eric.araujosetnosy: + eric.araujo
messages: + msg126089
2011-01-04 16:54:11georg.brandlsetnosy: + georg.brandl
messages: + msg125341
2011-01-04 15:01:57mher.movsisyansetfiles: + plist_validation_v2.diff
nosy: jackjansen, jvr, ronaldoussoren, ned.deily, mher.movsisyan
messages: + msg125333
2010-12-10 23:25:04ned.deilysetnosy: + ronaldoussoren, ned.deily

messages: + msg123758
stage: test needed -> patch review
2010-12-10 10:30:38mher.movsisyansetfiles: + plist_validation.diff

nosy: + mher.movsisyan
messages: + msg123725

keywords: + patch
2010-07-10 16:31:08BreamoreBoysetversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6
2009-04-22 17:16:45ajaksu2setkeywords: + easy
2009-02-14 12:49:24ajaksu2setstage: test needed
type: behavior
versions: + Python 2.6
2009-02-14 12:48:48ajaksu2setdependencies: + plistlib error handling
2004-07-04 21:30:29jackjansencreate