Closed Bug 710967 Opened 9 years ago Closed 9 years ago

Possible bug in AffixMgr::parse_convtable()

Categories

(Core :: Spelling checker, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: Dolske, Assigned: jaws)

References

Details

(Whiteboard: [pvs-studio][fixed-in-fx-team][fixed-in-hunspell-1.3.3])

Attachments

(1 file, 1 obsolete file)

From http://www.viva64.com/en/a/0078/

Example N1. Incomplete verification for table integrity

int  AffixMgr::parse_convtable(..., const char * keyword)
{
  ...
  if (strncmp(piece, keyword, sizeof(keyword)) != 0) {
      HUNSPELL_WARNING(stderr,
                       "error: line %d: table is corrupt\n",
                       af->getlinenum());
      delete *rl;
      *rl = NULL;
      return 1;
  }
  ...
}

PVS-Studio diagnostic message: V579 The strncmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. affixmgr.cpp 3708

The programmer tried to verify the table integrity here. Unfortunately, this check may both work and fail. To calculate the length of the key word the sizeof() operator is used, which is certainly incorrect. As a result, whether or not the code works will depend on pure luck (at certain values of the key word and 'keyword' pointer's size in the current data model).
Blocks: 710966
We can fix this locally, but I'm not sure how to go about getting these changes pushed upstream.
Assignee: nobody → jwein
Status: NEW → ASSIGNED
Attached patch Patch for bug 710967 (obsolete) — Splinter Review
I couldn't find a list of potential reviewers. Please redirect review request as necessary.

As far as the patch goes, the only callers of parse_convtable are internal to this file and only call this function with one of two constant arguments for |keyword| ("OCONV" or "ICONV"). As such, I'm not concerned about a buffer overrun here with the use of |strlen| since it is not using user-generated input.
Attachment #581914 - Flags: review?(gavin.sharp)
OS: Mac OS X → All
Hardware: x86 → All
Nemeth might be able to advise re: fixing this upstream.
Please update README.hunspell with the bug number for this fix as well.
Updated README.hunspell with the bug number.
Attachment #581914 - Attachment is obsolete: true
Attachment #581914 - Flags: review?(gavin.sharp)
Attachment #582101 - Flags: review?(gavin.sharp)
Attachment #582101 - Flags: feedback?(ryanvm)
Attachment #582101 - Flags: review?(gavin.sharp) → review+
Attachment #582101 - Flags: feedback?(ryanvm) → feedback+
https://hg.mozilla.org/integration/fx-team/rev/c6346cddd3c0
Whiteboard: [pvs-studio] → [pvs-studio][fixed-in-fx-team]
committed this to upstream hunspell now as well, thanks for this
https://hg.mozilla.org/mozilla-central/rev/c6346cddd3c0
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Whiteboard: [pvs-studio][fixed-in-fx-team] → [pvs-studio][fixed-in-fx-team][fixed-in-hunspell-1.3.3]
You need to log in before you can comment on or make changes to this bug.