Mercurial > p > roundup > code
comparison roundup/hyperdb.py @ 5257:928512faf565
- issue2550864: Potential information leakage via journal/history
Original code didn't fully implement the security checks.
Users with only Edit access on a property were not able to view the
journal entry for the property. This patch fixes that.
Also had additional info leakage: the target object of a link or
multilink must be viewable or editable in order for the journal entry
to be shown. Otherwise the existance of the target is exposed via the
journal while it is blocked from searches, direct access etc.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Sun, 27 Aug 2017 00:19:48 -0400 |
| parents | 198b6e810c67 |
| children | 29949f41cefb |
comparison
equal
deleted
inserted
replaced
| 5256:3639f4b55936 | 5257:928512faf565 |
|---|---|
| 1022 perm = self.db.security.hasPermission | 1022 perm = self.db.security.hasPermission |
| 1023 journal = [] | 1023 journal = [] |
| 1024 | 1024 |
| 1025 debug_logging = logger.isEnabledFor(logging.DEBUG) | 1025 debug_logging = logger.isEnabledFor(logging.DEBUG) |
| 1026 | 1026 |
| 1027 uid=self.db.getuid() # id of the person requesting the history | |
| 1028 | |
| 1027 for j in self.db.getjournal(self.classname, nodeid): | 1029 for j in self.db.getjournal(self.classname, nodeid): |
| 1030 # hide/remove journal entry if: | |
| 1031 # property is quiet | |
| 1032 # property is not (viewable or editable) | |
| 1028 id, evt_date, user, action, args = j | 1033 id, evt_date, user, action, args = j |
| 1029 if debug_logging: | 1034 if debug_logging: |
| 1030 j_repr = "%s"%(j,) | 1035 j_repr = "%s"%(j,) |
| 1031 else: | 1036 else: |
| 1032 j_repr='' | 1037 j_repr='' |
| 1033 if args and type(args) == type({}): | 1038 if args and type(args) == type({}): |
| 1034 for k in args.keys(): | 1039 for key in args.keys(): |
| 1035 if skipquiet and self.properties[k].quiet: | 1040 if skipquiet and self.properties[key].quiet: |
| 1036 logger.debug("skipping quiet property %s in %s", | 1041 logger.debug("skipping quiet property" |
| 1037 k, j_repr) | 1042 " %s::%s in %s", |
| 1038 del j[4][k] | 1043 self.classname, key, j_repr) |
| 1044 del j[4][key] | |
| 1039 continue | 1045 continue |
| 1040 if enforceperm and not perm("View", | 1046 if enforceperm and not ( perm("View", |
| 1041 self.db.getuid(), | 1047 uid, |
| 1042 self.classname, | 1048 self.classname, |
| 1043 property=k ): | 1049 property=key ) or perm("Edit", |
| 1044 logger.debug("skipping unViewable property %s in %s", | 1050 uid, |
| 1045 k, j_repr) | 1051 self.classname, |
| 1046 del j[4][k] | 1052 property=key )): |
| 1053 logger.debug("skipping unaccessible property %s::%s seen by user%s in %s", | |
| 1054 self.classname, key, uid, j_repr) | |
| 1055 del j[4][key] | |
| 1047 continue | 1056 continue |
| 1048 if not args: | 1057 if not args: |
| 1049 logger.debug("Omitting journal entry for %s%s" | 1058 logger.debug("Omitting journal entry for %s%s" |
| 1050 " all props quiet in: %s", | 1059 " all props removed in: %s", |
| 1051 self.classname, nodeid, j_repr) | 1060 self.classname, nodeid, j_repr) |
| 1052 continue | 1061 continue |
| 1053 journal.append(j) | 1062 journal.append(j) |
| 1054 elif action in ['link', 'unlink' ] and type(args) == type(()): | 1063 elif action in ['link', 'unlink' ] and type(args) == type(()): |
| 1064 # hide/remove journal entry if: | |
| 1065 # link property (key) is quiet | |
| 1066 # link property is not (viewable or editable) | |
| 1067 # id/object (linkcl, linkid) that is linked/unlinked is not | |
| 1068 # (viewable or editable) | |
| 1055 if len(args) == 3: | 1069 if len(args) == 3: |
| 1070 # e.g. for issue3 blockedby adds link to issue5 with: | |
| 1071 # j = id, evt_date, user, action, args | |
| 1072 # 3|20170528045201.484|5|link|('issue', '5', 'blockedby') | |
| 1056 linkcl, linkid, key = args | 1073 linkcl, linkid, key = args |
| 1057 cls = self.db.getclass(linkcl) | 1074 cls = self.db.getclass(linkcl) |
| 1058 if skipquiet and cls.properties[key].quiet: | 1075 # is the updated property quiet? |
| 1059 logger.debug("skipping quiet property %s in %s", | 1076 if skipquiet and self.properties[key].quiet: |
| 1060 key, j_repr) | 1077 logger.debug("skipping quiet property" |
| 1078 " %s::%s in %s", | |
| 1079 self.classname, key, j_repr) | |
| 1061 continue | 1080 continue |
| 1062 if enforceperm and not perm("View", | 1081 # property check on item we want history for |
| 1063 self.db.getuid(), | 1082 if enforceperm and not (perm("View", |
| 1083 uid, | |
| 1064 self.classname, | 1084 self.classname, |
| 1065 property=key): | 1085 property=key) or perm("Edit", |
| 1066 logger.debug("skipping unViewable property %s in", | 1086 uid, |
| 1067 key, j_repr) | 1087 self.classname, |
| 1068 continue | 1088 property=key)): |
| 1089 logger.debug("skipping unaccessible property %s::%s seen by user%s in %s", | |
| 1090 self.classname, key, uid, j_repr) | |
| 1091 continue | |
| 1092 # check on object linked | |
| 1093 if enforceperm and not (perm("View", | |
| 1094 uid, | |
| 1095 cls.classname, | |
| 1096 itemid=linkid) or perm("Edit", | |
| 1097 uid, | |
| 1098 cls.classname, | |
| 1099 itemid=linkid)): | |
| 1100 logger.debug("skipping unaccessible target %s%s for user%s in %s", | |
| 1101 cls.classname, linkid, uid, j_repr) | |
| 1102 continue | |
| 1069 journal.append(j) | 1103 journal.append(j) |
| 1070 else: | 1104 else: |
| 1071 logger.error("Invalid %s journal entry for %s%s: %s", | 1105 logger.error("Invalid %s journal entry for %s%s: %s", |
| 1072 action, self.classname, nodeid, j) | 1106 action, self.classname, nodeid, j) |
| 1073 elif action in ['create', 'retired', 'restored']: | 1107 elif action in ['create', 'retired', 'restored']: |