On Thu, Jun 4, 2015 at 5:43 PM, Peter Geoghegan <p...@heroku.com> wrote:
>
> BTW, there is a bug here -- strtol() needs additional defenses [1]
> (before casting to int):
>
> postgres=# select jsonb_set('[1, 2, 3, 4,
> 5,6,7,8,9,10,11,12,13,14,15,16,17,18]',
> '{"9223372036854775806"}'::text[], '"Input unsanitized"', false) ;
> jsonb_set
> ----------------------------------------------------------------------------------
> [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, "Input
> unsanitized", 18]
> (1 row)
>
> [1]
> https://www.securecoding.cert.org/confluence/display/cplusplus/INT06-CPP.+Use+strtol()+or+a+related+function+to+convert+a+string+token+to+an+integer
I attach a fix for this bug. The commit message explains everything.
--
Peter Geoghegan
From 2f2042d93d00f85e52612bd7d7499c3238579d4d Mon Sep 17 00:00:00 2001
From: Peter Geoghegan <peter.geoghega...@gmail.com>
Date: Fri, 12 Jun 2015 14:55:32 -0700
Subject: [PATCH 1/2] Fix "path" infrastructure bug affecting jsonb_set()
jsonb_set() and other clients of the setPathArray() utility function
could get spurious results when an array integer subscript is provided
that is not within the range of int.
To fix, ensure that the value returned by strtol() within setPathArray()
is within the range of int; when it isn't, assume an invalid input in
line with existing, similar cases. The path-orientated operators that
appeared in PostgreSQL 9.3 and 9.4 do not call setPathArray(), and
already independently take this precaution, so no change there.
---
src/backend/utils/adt/jsonfuncs.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index c14d3f7..13d5b7a 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3814,11 +3814,14 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
if (level < path_len && !path_nulls[level])
{
char *c = VARDATA_ANY(path_elems[level]);
+ long lindex;
errno = 0;
- idx = (int) strtol(c, &badp, 10);
- if (errno != 0 || badp == c)
+ lindex = strtol(c, &badp, 10);
+ if (errno != 0 || badp == c || lindex > INT_MAX || lindex < INT_MIN)
idx = nelems;
+ else
+ idx = lindex;
}
else
idx = nelems;
--
1.9.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
