Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard
verified publisher icon python-pillow

Memory leak bugs when a new reference is only passed to a non-stealing API (static analyzer reports)

Open Snape3058 opened this issue 3 years ago • 4 comments

  • API PyDict_SetItemString does not steal a reference from the third argument.
  • API PyDict_SetItem does not steal a reference from the last argument.
  • API PyList_Append does not steal a reference from the second argument.
  • API PyDict_SetItem does not steal a reference from the third argument if the return value is not zero.

If a new reference is passed to the function without decreasing its refcnt then, it will lead to a memory leak.

Pattern 1: APIs returning a new reference are called directly as the third argument.

  • Internal Report ID: 1b7403 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3773
  • Internal Report ID: 6fa78e https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3775
  • Internal Report ID: b43c87 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3777
  • Internal Report ID: 1640ab https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3779
  • Internal Report ID: 78d881 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3780
  • Internal Report ID: 6acded https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L3781
  • Internal Report ID: 15cbc4 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4154
  • Internal Report ID: a78d30 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4162
  • Internal Report ID: 446850 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4172
  • Internal Report ID: 3c8041 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4187
  • Internal Report ID: 1abe89 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4205
  • Internal Report ID: b9e74a https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4213
  • Internal Report ID: be23b5 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imaging.c#L4236
  • Internal Report ID: 611fdf https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1120
  • Internal Report ID: 7c0d23 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1121
  • Internal Report ID: b84747 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1123
  • Internal Report ID: 3c802a https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingmorph.c#L243
  • Internal Report ID: 71bbbc https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_webp.c#L960
  • Internal Report ID: 213466 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_webp.c#L949

Pattern 2: Intermediate variables are used to forward the argument.

  • Internal Report ID: e4a37b

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L1531 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L1535

  • Internal Report ID: dc5fd5

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L1533 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L1535

  • Internal Report ID: 1e988b

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L936 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L952

  • Internal Report ID: 0ef1f7

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L937 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingcms.c#L952

  • Internal Report ID: fce490

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1132 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1133

  • Internal Report ID: d17f14

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1347 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1348

  • Internal Report ID: 21a68f

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1368 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1372

  • Internal Report ID: e80f52

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1362 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1363 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1364 https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingft.c#L1365

  • Internal Report ID: 29a870

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingmorph.c#L195 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingmorph.c#L196

  • Internal Report ID: fddf55

New reference is returned here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingmorph.c#L231 PyObject is passed to non-stealing API here: https://github.com/python-pillow/Pillow/blob/68e39cba431680105c0e0f1a54371b7028e8da9a/src/_imagingmorph.c#L232

Snape3058 avatar May 23 '22 09:05 Snape3058

Would you be able to collect any more of these into a single issue?

radarhere avatar May 23 '22 09:05 radarhere

These are all the cases reported by my static analyzer. To find all of them, I suggest filtering and verifying the entire project for these four APIs.

Snape3058 avatar May 23 '22 09:05 Snape3058

Would you mind providing some detail on how you ran the static analyzer? What tool did you use specifically?

radarhere avatar May 23 '22 09:05 radarhere

It is an experimental analyzer of my unpublished research, which is developed on the top of the clang static analyzer.

Snape3058 avatar May 23 '22 14:05 Snape3058

I've created PR #7003 to resolve this.

API PyDict_SetItem does not steal a reference from the third argument if the return value is not zero.

To clarify, there's a copy paste error here. The text says "PyDict_SetItem", but the link is to "PyModule_AddObject". This statement applies to "PyModule_AddObject".

radarhere avatar Mar 11 '23 08:03 radarhere