Closed Bug 1140635 Opened 9 years ago Closed 9 years ago

Remove |magic_num| fields in sipcc

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: bwc, Assigned: bwc)

Details

Attachments

(1 file, 1 obsolete file)

The sipcc code has a bunch of |magic_num| fields on various structs that are used as a poor-man's UAF guard. They should be removed, since they could be masking problems.
Attached file MozReview Request: bz://1140635/bwc (obsolete) —
/r/5025 - Bug 1140635: Remove |magic_num| fields from sipcc.

Pull down this commit:

hg pull review -r bef61a38a8d0924ab4bd3d6236605fc4ed414ce9
Comment on attachment 8574211 [details]
MozReview Request: bz://1140635/bwc

For your viewing pleasure.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d62e45b1023
Attachment #8574211 - Flags: review?(martin.thomson)
Assignee: nobody → docfaraday
Comment on attachment 8574211 [details]
MozReview Request: bz://1140635/bwc

Check reviewboard.  I made some comments but they didn't get logged in bugzilla for some reason.
Attachment #8574211 - Flags: review?(martin.thomson)
Comment on attachment 8574211 [details]
MozReview Request: bz://1140635/bwc

/r/5025 - Bug 1140635: Remove |magic_num| fields from sipcc.

Pull down this commit:

hg pull review -r 26a9008c98c30ec48590dd68bc8ccb4acf4f7279
Comment on attachment 8574211 [details]
MozReview Request: bz://1140635/bwc

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40d2510b81c1
Attachment #8574211 - Flags: review?(martin.thomson)
Comment on attachment 8574211 [details]
MozReview Request: bz://1140635/bwc

https://reviewboard.mozilla.org/r/5023/#review4059

Unconditional r+.  How can I say no when you delete so much code.

::: media/webrtc/signaling/src/sdp/sipcc/sdp_attr_access.c
(Diff revision 2)
>          sdp_p->conf_p->num_invalid_param++;

1359 lines down, only 11031 to go.

::: media/webrtc/signaling/src/sdp/sipcc/sdp_attr_access.c
(Diff revision 2)
> -    if (sdp_verify_sdp_ptr(sdp_p) == FALSE) {
> +    attr_p = sdp_find_attr(sdp_p, level, cap_num, SDP_ATTR_FMTP, inst_num);
> +    if (attr_p == NULL) {
> +        if (sdp_p->debug_flag[SDP_DEBUG_ERRORS]) {
> +            CSFLogError(logTag, "%s fmtp attribute, level %u instance %u "
> +                      "not found.", sdp_p->debug_str, (unsigned)level, (unsigned)inst_num);
> +        }
> +        sdp_p->conf_p->num_invalid_param++;

Wow, either diff or reviewboard had a real brain fart right here.
Attachment #8574211 - Flags: review?(martin.thomson) → review+
Check back on try
Flags: needinfo?(docfaraday)
Seeing infra problems on try again. Retriggering some stuff, but there will be some delay.
Flags: needinfo?(docfaraday)
Flags: needinfo?(docfaraday)
https://hg.mozilla.org/mozilla-central/rev/e2f404ba1adf
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Flags: needinfo?(docfaraday)
Attachment #8574211 - Attachment is obsolete: true
Attachment #8619696 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: