Skip to content

Conversation

mruprich
Copy link
Contributor

When parsing PGM NCF packet type, the group AFNUM is extracted too late to be correct. In the switch in print-pgm.c under PGM_NCF option, when all the pointers are set, the pointers point to following:

struct pgm_header {                         <------ pgm points to the main PGM header
    nd_uint16_t pgm_sport;
    nd_uint16_t pgm_dport;
    nd_uint8_t  pgm_type;
    nd_uint8_t  pgm_options;
    nd_uint16_t pgm_sum;
    nd_byte     pgm_gsid[6];
    nd_uint16_t pgm_length;
};

struct pgm_nak {                     <----- nak points to (pgm + 1) that means it skips the main header and points here
    nd_uint32_t pgmn_seq;
    nd_uint16_t pgmn_source_afi;
    nd_uint16_t pgmn_reserved;
    /* ... uint8_t      pgmn_source[0]; */ <---- bp points to (nak + 1) so right after the first reserved part of the NAK header
    /* ... uint16_t     pgmn_group_afi */
    /* ... uint16_t     pgmn_reserved2; */
    /* ... uint8_t      pgmn_group[0]; */
    /* ... options */   
}

And then when the header is parsed, the pgmn_source is parsed, bp moves to pgmn_group_afi which is the value for the next switch but bp is again moved two bytes ahead before the group AFI can be extracted ( bp += (2 * sizeof(uint16_t)); ). 'switch (GET_BE_U_2(bp)) {' will be taking the value from pgmn_reserved2.

@mruprich
Copy link
Contributor Author

Hi, did anyone have time to take a look at this? Thanks.

@mruprich
Copy link
Contributor Author

mruprich commented Mar 2, 2023

Hi, any update with this PR? Anything I can do to help?

@mruprich
Copy link
Contributor Author

Hi again, is there anything wrong with this PR? Is something missing? Can I do something to get someone to take a look? Do you need some more info from me?

@fxlb
Copy link
Member

fxlb commented Apr 28, 2023

Thanks for the proposal. Before a review you should

  1. Use "git rebase" to rebase your work on top on the master branch.
    (See: CONTRIBUTING.md, item 8)
  2. Add a pcap file with at least a PGM NCF packet to show the effect of the change.
    (See: CONTRIBUTING.md, item 6)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants